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.

Sign up for GitHub

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

Jump to bottom

add CSS reset theme option #27517

Closed
wants to merge 1 commit into from
Closed

add CSS reset theme option #27517

wants to merge 1 commit into from

Conversation

scruffian
Copy link
Contributor

Description

WIP. This adds an option to experimental theme JSON to add a CSS reset

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@MaggieCabrera MaggieCabrera mentioned this pull request Dec 9, 2020
Closed
6 tasks
@oandregal
Copy link
Member

This is an interesting idea. I know that @youknowriad has mentioned in the past that we may want to add some kind of resets when the theme supports theme.json, but I don't know which specific styles he was referring to. Pulling his thoughts here because we may want to do this without the reset key.

@oandregal oandregal added Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress labels Dec 11, 2020
@youknowriad
Copy link
Contributor

I'm pretty sure that was not me :P

I personally don't like global resets and I'd love to understand the reasoning more :)

@oandregal
Copy link
Member

I'm pretty sure that was not me :P

😅 Ah, it was perhaps not related to resets, but outputting different CSS for some blocks (button)?

@oandregal
Copy link
Member

I only ask because if we have a list of things we want to do differently if the theme has a theme.json, we may consolidate the same mechanism for everything.

@scruffian
Copy link
Contributor Author

The reasoning is that there is a lot of boilerplate code that we ship with every theme, which could be reduced if it was simply part of the editor itself. @youknowriad what don't you like about them?

@youknowriad
Copy link
Contributor

youknowriad commented Dec 11, 2020 •
edited
Loading

Having a reset can break styles especially for things that are supposed to be "context agnostic". For example if you say box-sizing: border-box is part of the reset, blocks which styled are based on that assumption won't work on themes (or other contexts outside WP) that don't have that reset. For me, this kind of styles should be added to the blocks themselves and not assumed to be present. The theme shouldn't have to define the reset for these.

@scruffian
Copy link
Contributor Author

Makes sense. I'll close this one then :)

@scruffian scruffian closed this Dec 11, 2020
@scruffian scruffian deleted the add/css-reset-theme-option branch December 11, 2020 21:32
@carolinan carolinan mentioned this pull request Oct 8, 2021
Closed
@colorful-tones
Copy link
Member

Having a reset can break styles especially for things that are supposed to be "context agnostic".

I'm not sure if "break" is the right word. Alter might be more suitable. All CSS alters something.

supposed to be "context agnostic"

The context in our case: WordPress and/or even Gutenberg.

I think that it is likely that this discussion is a lot more nuanced than this and should be explored further.

Having a small, specific, per block reset seems fine. However, how would one reset the <body> element's margin?

Also, how would a per block reset look for List block and it's final output on the front end. Would the ul and ol be compounded and less performant? Whereas, having it declared and referenced once might be more performant? 🤔

It might be recommended that there be a combination of reset or normalization in WordPress core and Gutenberg. Ideally, both of them would be able to be dequeued by theme authors in theme.json.

I see that there is still ongoing discussion in #27116, which is encouraging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@oandregal oandregal Awaiting requested review from oandregal

@youknowriad youknowriad Awaiting requested review from youknowriad

@jasmussen jasmussen Awaiting requested review from jasmussen

Assignees
No one assigned
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@scruffian @oandregal @youknowriad @colorful-tones