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

FSE: CSS Reset #27116

Closed
scruffian opened this issue Nov 19, 2020 · 7 comments
Closed

FSE: CSS Reset #27116

scruffian opened this issue Nov 19, 2020 · 7 comments
Assignees
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@scruffian
Copy link
Contributor

In addition to the alignment styles that the front end should provide, we should also provide a CSS reset in Gutenberg to stop each theme having their own reset. Some stuff that is definitely needed:

img { height: auto; }
figure { margin: 0; }

Please add more as you find it :)

@MaggieCabrera
Copy link
Contributor

I'm going to add

pre {
	white-space: pre;
	overflow-x: auto;
}

to this list

@jasmussen
Copy link
Contributor

Cool ticket, I've been thinking about what to reset for a while now, how much, how little, and where. Some high level thoughts:

  • The iframe that @ellatrix has been working on, is likely to reframe aspects of efforts here.
  • We have some CSS reset stuff going on already, at the root of the block library:
  • style.scss has rules that apply to editor and frontend. You can see it includes a .aligncenter rule. I could swear we had rules for img as well, but they appear to be gone now — I'd recommend digging through the history for context.
  • editor.scss applies rules only to the editor (of course), and is good for any normalization stuff. We already have some there towards the bottom of the file.

Regarding this one specifically:

figure { margin: 0; }

Given how both images, embeds, audio and numerous other blocks now use figure as a wrapping element, I'm not sure adding this blanket rule is a good idea. Given its usage, I now think of this one more as like a p tag, one which needs careful top/bottom margins that work with the rhythm of the content in the post.

Furthermore, when the figure is inside the post content container, very often wide and fullwide is implemented using a combination of a max-width and margin-left: auto; margin-right: auto; (as seen here). By applying a blanket margin: 0;, you might break the centering of images and embeds inside post content. Something to test carefully.

@ellatrix
Copy link
Member

ellatrix commented Dec 1, 2020

I think we should go ahead with adding the iframe to the full site editor. Maybe this can be done this week?

@scruffian scruffian mentioned this issue Dec 4, 2020
6 tasks
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Dec 4, 2020
@ellatrix
Copy link
Member

Worth noting that the full site editor content is now iframed.

@scruffian
Copy link
Contributor Author

Are we expecting that the Post/Page editor will also become iframed?

@jasmussen
Copy link
Contributor

I'd love for that to happen. I also imagine a future where there isn't a separate site editor, but that template and site editing is integrated more directly as an option.

@youknowriad
Copy link
Contributor

I think this one is solved one way or the other.

  • figure resets are included in each block that uses "figure".
  • height: auto has been added where needed (when using width: 100%)
  • body margin is now reset.

It seems to me there's no need for reset anymore. I'm closing this issue for now, let me know if I missed anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants