pxToRem-settings #157
Open
pxToRem-settings #157
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andypost
reviewed
Apr 26, 2022
| '!filter', | ||
| '!border', | ||
| '!border-width', | ||
| '!border-top-width', |
There was a problem hiding this comment.
Please sort the list alphabetically
koskinpark
reviewed
Apr 27, 2022
| @@ -27,9 +27,30 @@ module.exports = () => ({ | |||
| }), | |||
| postcssDiscardEmpty(), | |||
| pxtorem({ | |||
| minPixelValue: 3, | |||
There was a problem hiding this comment.
i'm not sure all these changes are useful..
usually we use rems scaling. See example: on 1440px design you have border with 2px width. But what you will show on retina 5120px resolution? Your 2px will look tiny. That's why everything should be converted to rems
There was a problem hiding this comment.
For the CSS which is really required to be in pixel, you can use the following code:
.selector {
/* stylelint-disable-next-line unit-case */
width: 2PX;
}
In that case it will not be automatically converted to rems
There was a problem hiding this comment.
i would better set fixed base font size after 1920
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upd.
After discussions and Drupal best practices: we are going to convert into rem all.
Some cases where need to be used line in 1px - it should be done by border.
====
Setted ignore list of styles, like border etc.
Proposition for minimum px to covert = starts from 3px.
It minimizes adding everywhere exclusion for the linter and for the calculation of the CSS.
Proposition to disable unit case check and linter fix.
inside file ".stylelintrc"
add "unit-case": null,
Also, to minimize linter exclusion of unit-case in the code.
I don't think that converting 1px or even 2px sometimes is a good idea, also add every time exclusion for the border: 1px;
We can disable this globally.
Where we need this in rem's - we can convert it manually.