Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First cut of selector optimization (issue #100) #114

Closed
wants to merge 5 commits into from

Conversation

karthikv
Copy link
Contributor

As per my description on issue #100, selectors like these:

.topbar form input
.pagination ul li a
.topbar ul li ul li a
.tabs li a

should be replaced by more efficient and simplified counterparts that don't have prodigal descendants:

.topbar input
.pagination ul a /* or */ .pagination li a
.topbar ul ul a /* or */ .topbar li li a
.tabs a

The commits associated with this pull request achieve many optimizations similar to the ones above in all LESS files that need them. In addition to the aforementioned examples, selectors with non-generic class names as descendants, like so:

.modal .modal-body
table .headerSortUp

have also been optimized by simply removing the parent selector:

.modal-body
.headerSortUp

This has only been done where class names are unique and quite unlikely to be re-used elsewhere.

In terms of statistics, five of the eight LESS files benefited from these optimizations, many in a significant manner. More than just greater efficiency, this also cut down about 700 bytes (~2%) from the minified stylesheet. Considering that the only change was to selectors, I would say this is a nice bonus.

Even though this takes care of many optimizations, there are still various methods to accomplish even more. For example, because a decent number of class names are generic, the selectors including them require more context and therefore more complexity. Changing the markup to supplant optimization and circumvent these cases is a frontier not explored in these commits. Re-thinking the CSS structure as a whole can also result in insights of what can be consolidated. For these reasons, I ask that you continue to keep issue #100 open so that further optimization—and discussion regarding it—is continually strived for.

@fat
Copy link
Member

fat commented Aug 28, 2011

Wow this is great stuff! I'll review more tomorrow, but it looks awesome! Thanks :D

@mdo
Copy link
Member

mdo commented Aug 28, 2011

Holy underwear! That's a lot of changes. Good work, sir. I'll dig in and give this bad boy a go.

@ghost
Copy link

ghost commented Aug 29, 2011

Wow, great stuff. This is how you do a pull request!

@mdo
Copy link
Member

mdo commented Aug 30, 2011

Sweet baby jeebus. We're working on merging a lot of this by hand right now—most was able to get most of the files merged without a problem, but the patterns gave us shit after I refactored the dropdowns. More to come on this!

@karthikv
Copy link
Contributor Author

No rush! I'm glad you're incorporating it. Let me know if you find any issues that I need to fix.

@mdo
Copy link
Member

mdo commented Sep 2, 2011

Closing this—we merged karthikv's pull in awhile back and have made many updates since then for the upcoming release. More to come when that drops.

@mdo mdo closed this Sep 2, 2011
DocX pushed a commit to DocX/bootstrap that referenced this pull request Sep 16, 2014
Documentation: how to use Bootstrap mixins with multiple values
cvrebert added a commit that referenced this pull request May 29, 2015
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