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

Fix a typo in classic block, and scope Quote editor styles #7040

Merged
merged 4 commits into from
May 31, 2018

Conversation

jasmussen
Copy link
Contributor

This PR does two things

  1. It fixes a typo in the classic block

  2. It scopes the editor quote styles to truly be editor-only.

Number 2 is important for implementations that might load Gutenberg itself on the front-end. Scoping the styles to be in-editor helps prevent CSS bleed to outside the editing canvas.

I would like your thoughts on this, because if 2 is approved, then perhaps we should use the same pattern for all other blocks. Right now were using a mixture of .editor-block-list__block[data-type="core/blockname"] and .wp-block-blockname. If every editor style was scoped with the former, we'd truly keep it in-canvas, enhancing reusability.

Let me know if this is a good plan, and I'll expand this PR with a few commits.

By the way this PR was done "blind", because I'm blocked on what I think is an issue with the build process currently: #7027 (comment)

@jasmussen jasmussen added the [Type] Enhancement A suggestion for improvement. label May 31, 2018
@jasmussen jasmussen self-assigned this May 31, 2018
@@ -1,12 +1,13 @@
.wp-block-quote {
.editor-block-list__block[data-type="core/image"] .wp-block-quote {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like a bug here "core/image"? Also, be mindful that we should try to avoid these selectors because the .editor-block-list__block[data-type="core/*"]div is not rendered when you save the block as reusable breaking the styles when editing reusable blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely a bug. Fixed typo, thanks.

Also, be mindful that we should try to avoid these selectors because the .editor-block-list__block[data-type="core/*"]div is not rendered when you save the block as reusable breaking the styles when editing reusable blocks.

Right — this would be for scoping editor-only styles. Any style that should be shown in both the editor and the front-end should presumably be in style.scss right?

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 Riad is trying to explain that when you save your block as reusable (Saved) block, those rules won't be applied, because it is going to be controlled by data-type="data" matcher.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmmmmm. Okay, tricky. I'll remove this for now, but it'd be nice if there was a way to scope styles to be editor only.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we use .gutenberg className to scope those for now.

@jasmussen
Copy link
Contributor Author

Reverted the scoping for now. This is now just a classic editor typo fix.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Type change itself looks good.

@jasmussen jasmussen added this to the 3.0 milestone May 31, 2018
@jasmussen jasmussen merged commit c591184 into master May 31, 2018
@jasmussen jasmussen deleted the polish/classic-and-quote branch May 31, 2018 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants