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

chore: sort Sass components in categories #10856

Merged

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Jan 11, 2018

Closes:

Currently, import/includes in foundation.scss are sorted alphabetically. This is a very bad idea.

Import/includes order in Sass matters a lot as the last CSS properties declared will override the first ones. So components should be sorted in a more "generic" to more "specific" components. When a component relies on others (like accordion-menu with menu), these dependencies should be imported and included before.

Changes: Sort CSS includes in the following categories:

  1. Global styles
  2. Generic components (controls and HTML-oriented components)
  3. Basic components (components with enclosed layout styles)
  4. Containers (components with side-effects on internal layout)
  5. Layout (components with side-effects on external layout)
  6. Helpers (highly specific and non-inheritable classes)

⚠️ Breaking changes MAY HAVE been introduced. Tests are required to ensure it did not created bugs.

Sort CSS includes in the following categories:
1. Global styles
2. Generic components (controls and HTML-oriented components)
3. Basic components (components with enclosed layout styles)
4. Containers (components with side-effects on internal layout)
5. Layout (components with side-effects on external layout)
6. Helpers (highly specific and non-inheritable classes)
@ncoden
Copy link
Contributor Author

ncoden commented Jan 30, 2018

@colin-marshall @brettsmason Could you review this ?
(also poke @SassNinja)

@ncoden
Copy link
Contributor Author

ncoden commented Mar 3, 2018

I checked all component and everything looks fine.
@SassNinja Could you make a review ?

@colin-marshall
Copy link
Contributor

@ncoden I like this better, but it seems the categories themselves would be more organized if it could be sorted alphabetically when dependencies don't matter. For example, I think drilldown-menu should come before dropdown-menu. Another example is the sorting of generic components category looks to be completely random, but maybe there are dependencies I'm not seeing.

@ncoden
Copy link
Contributor Author

ncoden commented Mar 5, 2018

@colin-marshall You are right. I'll change that.

Note:
* forms & typograph has been moved to global styles, as they apply properties on tags and not custom components
* Accodtion/Drilldrown/Dropdown menus must be after `menu`
@ncoden
Copy link
Contributor Author

ncoden commented Mar 23, 2018

I resolved conflicts with develop, moved typography and forms to the "Global styles" category, and sorted components inside categories alphabetically.

@ncoden ncoden force-pushed the chore/sort-sass-components-by-specificity branch from 54074cd to fecf67d Compare March 26, 2018 17:46
@ncoden ncoden merged commit ff43dde into foundation:develop Mar 26, 2018
@ncoden ncoden deleted the chore/sort-sass-components-by-specificity branch April 7, 2018 17:41
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
…nts-by-specificity for v6.5.0

7ab9707 chore: sort CSS includes by specificity
952c9c6 docs: update list of Sass includes
fecf67d style: sort Sass components inside each category alphabetically

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants