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

CSS combing #1

Closed
raphaelgoetter opened this issue Jul 28, 2017 · 11 comments
Closed

CSS combing #1

raphaelgoetter opened this issue Jul 28, 2017 · 11 comments
Labels
planning Discussion about the project's upcoming features and more.

Comments

@raphaelgoetter
Copy link

Hi,

First sorry for my bad english :)

I really love your project but I notices something that bothers me a bit: the CSS properties order is sometimes strange and could be fixed with a tool like CSScomb

For example : here you could reorder this :

pre {
  max-height: var(--pre-max-height);
  overflow-y: auto;
  font-family: var(--font-family-monospace);
  color: var(--pre-color);
  background-color: var(--pre-bg-color);
  border-radius: var(--pre-border-radius);
  padding: 1rem;
  margin-top: 0;
  margin-bottom: 1rem;
}

into this :

pre {
  overflow-y: auto;
  max-height: var(--pre-max-height);
  margin-top: 0;
  margin-bottom: 1rem;
  padding: 1rem;
  color: var(--pre-color);
  border-radius: var(--pre-border-radius);
  background-color: var(--pre-bg-color);
  font-family: var(--font-family-monospace);
}

And that would be even geater :)

@claviska
Copy link
Member

Yes, this bothers me too. I was just having a conversation with @orokusaki about this the other day. I have my own "method" but it's often inconsistent.

What's the preferred sort order using CSS comb? Is there a default? The website doesn't seem to describe one and the example above seems a bit arbitrary at a glance.

Also, the minified stylesheet is obviously irrelevant, so this will only apply to the source files. I can't see how it would be part of the build process, so are we supposed to use .csscomb.json + an editor plugin such as this one?

@raphaelgoetter
Copy link
Author

raphaelgoetter commented Jul 28, 2017

CSS Combs default order is ZenCoding (Found it in this CSS tricks article.)

this will only apply to the source files.

Of course, but a clean source code (with editorconfig, beautify and CSS comb) is a pleasure to deal with :)

@raphaelgoetter
Copy link
Author

Found a better resource for orders : https://github.com/csscomb/csscomb.js/tree/master/config

@claviska claviska added the planning Discussion about the project's upcoming features and more. label Jul 28, 2017
@claviska
Copy link
Member

I think the reason they don't recommend an Atom plugin on the CSScomb website is because none of them work very well. I've tried four different ones. Most don't respect my .csscomb.json file despite it being in the prefs. The only one that does throws errors for valid CSS. Here's an example:

.button {
  padding: calc(var(--component-spacing) * .5) calc(var(--component-spacing) * .75);
}

screen shot 2017-07-31 at 11 04 09 am

Strangely enough, the "erroneous" line works fine in the CSScomb demo.

So I'm not sure if this is an issue with CSScomb itself or the Atom plugin. 4.2.0 is the latest release of CSScomb, so it's not a version issue at least.

End of day: I like the idea but I'm not going to switch my editor just to sort some CSS props. If I can find an Atom plugin that honors .csscomb.json properly, I'm happy to use it. Until then: 🤷🏻‍♂️

Here's my generated config for future reference:

{
  "remove-empty-rulesets": true,
  "always-semicolon": true,
  "color-case": "lower",
  "block-indent": "  ",
  "color-shorthand": true,
  "element-case": "lower",
  "eof-newline": true,
  "leading-zero": false,
  "quotes": "single",
  "sort-order-fallback": "abc",
  "space-before-colon": "",
  "space-after-colon": " ",
  "space-before-combinator": " ",
  "space-after-combinator": " ",
  "space-between-declarations": "\n",
  "space-before-opening-brace": " ",
  "space-after-opening-brace": "\n",
  "space-after-selector-delimiter": "\n",
  "space-before-selector-delimiter": "",
  "space-before-closing-brace": "\n",
  "strip-spaces": true,
  "tab-size": true,
  "unitless-zero": true
}

@claviska claviska changed the title CSS combing ? CSS combing Aug 1, 2017
@pkill37
Copy link

pkill37 commented Aug 2, 2017

Maybe you could run https://github.com/csscomb/csscomb.js in a git pre-commit hook or something like that.

@thomasbritton
Copy link

thomasbritton commented Aug 3, 2017

Personally, I think just sorting the properties alphabetically would be the best option if you really want to spend the time putting an ordering system in place. It's the simplest option to update the existing source code to and will be the most universally understood ordering.

But at the end of the day, I don't think people should be editing the CSS provided from Shoelace anyway, it's meant to be a reset/normalise framework and if you need/want to make adjustments you should do so outside of the core CSS so it doesn't really matter how you order the properties, not to mention that all of the projects CSS should be minified anyway so what benefit is it really giving the user?

This is certainly one of those 'Nice to haves' rather than being a 'Should have'.

@claviska
Copy link
Member

claviska commented Aug 3, 2017

Sorting alphabetically makes it easier to add/remove/order properties when you're developing, but I tend to group things (e.g. font/text styles, positioning styles, etc.) so I don't have to bounce around a lot. CSScomb's default order seems logical enough — I just with I could get it to work properly with a plugin.

Agree that this is a "nice to have" and I'll hold off and maybe try to troubleshoot one of those plugins again later.

@TheMonster1995
Copy link

TheMonster1995 commented Aug 7, 2017

I beg to differ on the matter of it being a "nice to have." If this is going to be an open source project which many people are going to use, enjoy, and contribute to, then the core code should be the easiest to understand; whether it's for debugging purposes, or just someone trying to add something new and wants to understand the other parts of the code.

Also, if different people are going to contribute to this project, a considered standard for this and similar matters would make the contribution easier, and prettier.

I truly believe this actually is a "should have."

@claviska
Copy link
Member

claviska commented Aug 9, 2017

Also tried postcss-sorting for Atom. This mostly worked, but there are some annoying bugs that cause it to use the wrong config for files opened outside the current project.

I'll keep looking for in-editor solutions. Hoping for one that:

  • Uses a config file such as .csscomb.json or .postcss-sorting.json
  • Has the ability to sort manually
  • Has the ability to auto-sort on save
  • Doesn't expand one-liners such as these

@claviska
Copy link
Member

I added stylelint to the project. This catches a lot of convention issues, but it doesn't handle sorting. Still haven't found anything that works properly in Atom.

@claviska
Copy link
Member

claviska commented Jul 3, 2020

In the years since this issue was created, I've still not managed to find a CSS "comb" that works the way I like. I'm closing this as a wontfix in prep for the 2.0 release, but if I come across something that works well I may add it later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
planning Discussion about the project's upcoming features and more.
Projects
None yet
Development

No branches or pull requests

5 participants