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

Enhance css reset #211

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Enhance css reset #211

wants to merge 2 commits into from

Conversation

joesnellpdx
Copy link
Contributor

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

  • Numbered the order of files - for more clear hierarchy and compilation sequence
  • Added a 01-Settings directory (including index.css, colors,css, custom-selectors.css, and media-queries.css
  • The previously mentioned custom-selectors.css file was added to enable bulk selectors
  • In 02-Global, index.css, mixins.css, and the new reset.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 reset
  • color-scheme: dark light; to html - add default color schemes to project - available if needed
  • hanging-punctuation: first last; - enhance block quotes or large content blocks in quotes
  • body { min-height: 100svh; } ensure body is at least 100% of viewport hight (including on mobile devices)
  • Responsive media: set to display: block; and max-width: 100%; by default
  • All headings: add text-wrap: balance allow browser to set a balanced heading if more than one line
  • All paragraphs: add text-wrap: pretty ensure there are no orphans on paragraphs that are more than one line
  • Add smooth scrolling by default

These 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

  1. All text will be the same font (style, weight, etc)
  2. No margin or padding will exist
  3. You can add @media (prefers-color-scheme: [dark or light]) {} media queries to test styles based on chosen color scheme
  4. You can add a large paragraph wrapped in quotes, and the quotes to hang (text vertically aligned left and right) by default
  5. If you select the body element - it will grow to fit the entire viewport height - even on mobile devices
  6. This should be basic
  7. Headings should evenly balance across multiple lines (on supporting browsers)
  8. Paragraphs should not leave an orphan word when spanning multiple lines (on supporting browsers)
  9. Smooth scrolling will animate the browser scrolling when a user clicks an element linking to a different section on the same page. This will also work within modals.

Changelog Entry

Added - Custom CSS Reset enhancements, new CSS folder, custom-selectors.css
Changed - Existing CSS directories in frontend

Credits

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

- refactor css file structure organizatoin
- add new utilities for css
@joesnellpdx joesnellpdx self-assigned this Feb 9, 2024
Copy link
Member

@fabiankaegy fabiankaegy left a 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.

  1. Settings: global variables like fonts and colors.
  2. Tools: mixins and functions.
  3. Generic: Resets and box-sizing.
  4. Elements: unclassed HTML elements like <h1> and <blockquote>.
  5. Layouts (objects): undecorated design patterns, such as global layouts and wrappers.
  6. Blocks: styles for Core and custom blocks.
  7. Components: styles for components, such as navigation and pagination.
  8. Custom layer: If there is need for custom layer, feel free to add it in. It’s OK to be before blocks and components.
  9. Utilities: Utility classes which overwrites previous layers styles, like .screen-reader-text and prefers-reduced-motion.

* Ensure body is at least 100% of viewport height
*/
body {
min-height: 100svh;
Copy link
Member

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?)

Suggested change
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 */
Copy link
Member

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.

Copy link
Member

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 */
Copy link
Member

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;
Copy link
Member

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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 */
Copy link
Member

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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 */

Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance CSS reset with a custom reset.css file
3 participants