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

Update editor classes to match what's currently in Gutenberg #686

Merged
merged 1 commit into from
Jan 11, 2020

Conversation

laurelfulford
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

As of Gutenberg 7.2, the legacy classes .editor-block-list__layout and .editor-block-list__block were removed from the editor.

These are currently used in the editor for the custom colours and typography; this PR updates them to their replacement classes .block-editor-block-list__layout and .block-editor-block-list__block

How to test the changes in this Pull Request:

  1. Verify that you're running Gutenberg 7.2.
  2. Under the Customizer, apply custom colours and fonts.
  3. View a few posts/pages in the editor; note that the colours/fonts are not being applied.
  4. Apply the PR.
  5. Confirm that your custom colours/fonts are now being applied.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@laurelfulford laurelfulford added the [Status] Needs Review The issue or pull request needs to be reviewed label Jan 9, 2020
@jeffersonrabb
Copy link
Contributor

It looks like this is about replacing all instances of .editor-block-list__layout with .block-editor-block-list__layout and all instances of .editor-block-list__blockwith .block-editor-block-list__block. Based on this assumption, I searched the codebase and found a few occurrences of editor-block-list__layout and .editor-block-list__block, all in the editor stylesheets for various stylepacks. Is this anything to worry about? (Maybe not, given that we're working on sunsetting these stylesheets anyway?)

@@ -380,93 +380,93 @@ function newspack_custom_colors_css() {
* - buttons
*/

.entry-meta .byline a {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only change that doesn't appear to be a replacing editor-block-list__layout .editor-block-list__block. Is it correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's that, and here: https://github.com/Automattic/newspack-theme/pull/686/files/a7eca439d09f988db35e07de3fceaeba36c4f66f#diff-38fb28a9c52ba7b44eaa303759a7aaadL363

Both added the .block-editor-block-list__layout .block-editor-block-list__block classes where there weren't any before, for consistency; for the second change, there was also a missing , after the .cat-links selector, causing some of the text-transform: uppercase styles not to work (something I found when testing this). Apologies, I should've called that out more clearly!

@jeffersonrabb jeffersonrabb added [Status] Needs Changes or Feeback The issue or pull request needs action from the original creator and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Jan 10, 2020
@laurelfulford
Copy link
Contributor Author

Based on this assumption, I searched the codebase and found a few occurrences of editor-block-list__layout and .editor-block-list__block, all in the editor stylesheets for various stylepacks. Is this anything to worry about? (Maybe not, given that we're working on sunsetting these stylesheets anyway?)

Gah, good catch! Those should be replaced, too -- I was focused on the colours/typography because they were the parts I noticed that had stopped working, I should have checked the other stylesheets, too. Fresh commit to fix that is incoming!

@laurelfulford
Copy link
Contributor Author

Apparently I can't read today 🤦‍♀ You're right, those styles won't be needed anymore as they are now:

#685 is a theme PR that makes tweaks to the columns block, to go with changes being made to that markup. As part of it, I was able to simplify the editor styles, and remove the .editor-block-list__layout and .editor-block-list__block classes entirely. I think it probably makes sense to leave them as is with this PR, and let that one take care of those files, to avoid conflicts.

Just let me know if you think that's a bad idea, or if you have any questions about this!

@laurelfulford laurelfulford added [Status] Needs Review The issue or pull request needs to be reviewed and removed [Status] Needs Changes or Feeback The issue or pull request needs action from the original creator labels Jan 11, 2020
Copy link
Contributor

@jeffersonrabb jeffersonrabb 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 explaining all of that - looks good.

@jeffersonrabb jeffersonrabb added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Jan 11, 2020
@laurelfulford
Copy link
Contributor Author

Thanks Jeff! 🙌

@laurelfulford laurelfulford merged commit df99310 into master Jan 11, 2020
@laurelfulford laurelfulford deleted the updates/gutenberg-editor-classes branch January 11, 2020 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants