Skip to content

Conversation

@talbs
Copy link
Contributor

@talbs talbs commented Oct 16, 2013

This LMS work helps to:

  • resolve a known IE9 CSS Selector limit bug (LMS-1020- https://edx-wiki.atlassian.net/browse/LMS-1020).
  • provides better, more consistent names to our outputted CSS files
  • move vendor-related CSS to be loaded when needed (overall vs. in-courseware)
  • move IE-specific styling (largely left untouched) into using standard body element classes
  • FYI - My editor also stripped out some whitespace as part of my editing

This work brought up large concerns:

  1. This resolves the bug noted above, but is far from ideal. Given that the LMS Sass architecture (the individual files, the inheritance/dependency within them, and the unorganized styling across the app), I could not find a logical/semantic way to segment the application.scss (generated by application.scss.mako for alpha-level theming reasons). Thus the separate CSS files are suffixed with -extend1 and -extend2.
  2. Also, there is a ton of redundancy where each application(-extend*).css file is calling the same Sass set-up files, generating redundant styling rules that are already cached in its sibling files (essentially, we're making the browser load/parse a subset of the same rules in two separate stylesheets - not great at all).

These two points really concern me and I highly recommend a very deep revamp/cleaning of the Sass architecture shortly. I've tested the display of many pages locally, but each page's display should really be stress-tested to ensure there are no visual regressions. If you want to test the number of rules being used, crack open your console and run this gist in a view - https://gist.github.com/psebborn/1885511

Per Rob/Miki, its been decided to merge this change into the release branch.

brianhw added a commit that referenced this pull request Oct 16, 2013
LMS: Revised CSS Architecture (resolves LMS_1020)
@brianhw brianhw merged commit 291db0a into rc/2013-10-17 Oct 16, 2013
@brianhw brianhw deleted the talbs/lms-cssarch branch October 16, 2013 17:19
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Oct 27, 2016
…tyle_reply_message_for_forum

Fix font size that reply comments for discussion forum openedx#1362
john2x pushed a commit to open-craft/openedx-platform that referenced this pull request Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants