-
Notifications
You must be signed in to change notification settings - Fork 50
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
Enhance css reset #211
base: trunk
Are you sure you want to change the base?
Enhance css reset #211
Conversation
- refactor css file structure organizatoin - add new utilities for css
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.
Thanks for creating this and improving the base setup we have here.
I added a bunch of little comments with my thoughts inline.
On a high level I would also love to rethink the actual folder structure we have setup here. I think the folder structure as we have it actively works against the CSS cascade in many ways because the order of how styles are loaded is somewhat random.
I'm still a big fan of ITCSS as described by Sami Keijonen all the way back in 2019 https://foxland.fi/maintainable-css-architecture-in-the-gutenberg-era/
The reason for this is that I want all of my utility styles (.has-font-size-large
) to be imported at the very end of the main stylesheet so that they automatically override anything else with the same low specificity.
- Settings: global variables like fonts and colors.
- Tools: mixins and functions.
- Generic: Resets and box-sizing.
- Elements: unclassed HTML elements like
<h1>
and<blockquote>
. - Layouts (objects): undecorated design patterns, such as global layouts and wrappers.
- Blocks: styles for Core and custom blocks.
- Components: styles for components, such as navigation and pagination.
- Custom layer: If there is need for custom layer, feel free to add it in. It’s OK to be before blocks and components.
- Utilities: Utility classes which overwrites previous layers styles, like
.screen-reader-text
andprefers-reduced-motion
.
* Ensure body is at least 100% of viewport height | ||
*/ | ||
body { | ||
min-height: 100svh; |
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.
Shouldn't we be using dvh
here? (And provide a fallback for when the new unit isn't available?)
min-height: 100svh; | |
min-height: 100%; | |
min-height: 100dvh; |
html { | ||
|
||
/* enable dark and light color schemes by default */ | ||
color-scheme: dark light; /* stylelint-disable-line scale-unlimited/declaration-strict-value */ |
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.
Do we want to enable dark mode by default? We don't usually style for it unless it is a specific requests.
Also I don't think we should have to override our own linting rules in our scaffold.
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.
I don't think this enables dark mode by default, just that it allows it to be both dark and light
color-scheme: dark light; /* stylelint-disable-line scale-unlimited/declaration-strict-value */ | ||
|
||
/* enable hanging punctuation */ | ||
hanging-punctuation: first last; /* stylelint-disable-line scale-unlimited/declaration-strict-value */ |
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.
Same thing here with the stylelint override
picture, | ||
svg, | ||
video { | ||
display: block; |
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.
I'm a strong believer in setting images to display: flex
in order to make them loose that odd additional spacing they always have at the bottom.
} | ||
|
||
:--headings { | ||
text-wrap: balance; |
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.
text-wrap: balance; | |
text-wrap: balance; | |
text-wrap: pretty; /* pretty is the nicer of the two algorythems. But it has less browser support so we want to have balance as a fallback */ |
* | ||
* NOTE: if using the lobomized owl technique - this style may be overridden | ||
*/ | ||
max-width: 75ch; /* 75 characters - may wish to replace with custom property */ |
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.
I think this is too opinionated for the scaffold
* NOTE: if using the lobomized owl technique - this style may be overridden | ||
*/ | ||
max-width: 75ch; /* 75 characters - may wish to replace with custom property */ | ||
text-wrap: pretty; |
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.
text-wrap: pretty; | |
text-wrap: balance; | |
text-wrap: pretty; /* pretty is the nicer of the two algorythems. But it has less browser support so we want to have balance as a fallback */ |
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 has a negative impact on performance, while I was hesitant on all headings by default, definitely shouldn't happen on all paragraphs
Description of the Change
This pull request adds a new CSS file named
reset.css
. This file is used to enhance our existing reset.Some file structure names were altered and names changed for the primary files in
frontend.css
01-Settings
directory (includingindex.css
,colors,css
,custom-selectors.css
, andmedia-queries.css
custom-selectors.css
file was added to enable bulk selectors02-Global
,index.css
,mixins.css
, and the newreset.css
were added.Styles added to the
reset.css
file:font: inherit
globally to cover for some browser inconsistencies and provide more font display reliability-- NOTICE: this will flatten all browser font styles and rely 100% on styles coming from the site
margin: 0; padding: 0;
globally to ensure there is a reliable spacing resetcolor-scheme: dark light;
to html - add default color schemes to project - available if neededhanging-punctuation: first last;
- enhance block quotes or large content blocks in quotesbody { min-height: 100svh; }
ensure body is at least 100% of viewport hight (including on mobile devices)display: block;
andmax-width: 100%;
by defaulttext-wrap: balance
allow browser to set a balanced heading if more than one linetext-wrap: pretty
ensure there are no orphans on paragraphs that are more than one lineThese styles were tested locally and work as expected.
Closes #210
How to test the Change
Once styles are loaded into a project, you can test each enhancement individually.
Assuming no custom styles added
@media (prefers-color-scheme: [dark or light]) {}
media queries to test styles based on chosen color schemeChangelog Entry
Credits
Checklist: