-
-
Notifications
You must be signed in to change notification settings - Fork 79.1k
Docs v5: Fix Sass rounding precision and state Sass implementation #32512
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was intentional, though. Please just keep the doc change to read 10
instead of 6
.
@XhmikosR The only discussion on the topic I could find (linked in previous comment) made it seem like the intention was to still lower the precision to 6, at least for minified CSS. Is the precision to always be left at 10, then? If so I'll adjust the doc to read such. Do you recall the reasoning why we're not utilizing the free reduction in characters, so that I can properly reword the doc explanation? |
We din't want to change the default output precision, at least not yet. Please only update the docs. |
a5d1128
to
56fe4f0
Compare
I've reverted the minification script changes and updated the doc to reflect the current build setup. |
I've expanded the explanation and added a mention of LibSass being deprecated |
0a01fbc
to
5dcaefb
Compare
This feels like an odd thing to put here in our customization section. I'm not entirely sure why there's anything there about the precision even now. I don't feel like this is something really worth documenting honestly? |
@mdo the current sentence is wrong right now:
|
Yeah I'd just rip all that out. Doesn't make sense to mention it at all tbh. |
Your call, feel free to edit this PR or make a new one then. BTW we do have an issue about mentioning the Sass implementation we support. #29760 |
For what is worth, I personally don't mind keeping this PR as is. It explains why people shouldn't lower the precision since we have been historically bit by that. |
The Sass implementation the framework is built with and the precision used when building and/or minifying the CSS are both required knowledge to setup a build script when customizing Bootstrap. It would be nice to not have to read through the npm scripts just to gather this knowledge as a user of the framework, and if I were a new user to the framework I think that Docs > Customize > Sass is a reasonable page to navigate to to look for such things. Perhaps the placement within the page isn't ideal, but I think just giving it a heading of "Building" would help clarify what the paragraphs are about and further help a new user find what they're looking for quickly and where they might expect the information to live. Edit: I've added the heading and I think the placement of the paragraphs makes more sense now. Also the info is now even more easily found. |
@XhmikosR Would you like me to create a similar PR for v4 that states the Sass implementation used in order to help close out #29760? If so, should that one also note that Node Sass is deprecated, or just state that its the implementation used and leave it at that (since it wasn't deprecated when v4 was created)? |
976e87c
to
d787c98
Compare
d787c98
to
9a70c24
Compare
@voltaek sorry for the delay, it's holidays + quarantine. We'll get to do it soon :) |
…mentation used, mention current precision of 10 due to Dart Sass, and mention recommended minimum precision (value of 6 used in BS v4). Remove outdated Sass precision from Customize > Sass docs page.
9a70c24
to
53736cd
Compare
… Sass compiler mentions so we can keep all pertinent Sass compiler information in just one location instead of spread through the docs, which makes it hard to find the details and/or keep them up-to-date.
I'm going to merge this @voltaek, can you please backport this to v4? Without dart-sass since we use Node sass there. |
The switch from Node Sass to Dart Sass has ended up with all the output CSS having Dart Sass's fixed precision of 10. As mentioned in the current docs and also here, the intended final precision is 6, not 10. As such I've adjusted the parameters of both npm minification scripts to round to a precision of 6.
I also updated the site doc's blurb about precision values used with SASS building to reflect the current precisions used.