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

Jump to bottom

Fix Blocks list ordering in Global Styles #39093

Merged
merged 1 commit into from Mar 7, 2022
Merged

Fix Blocks list ordering in Global Styles #39093

merged 1 commit into from Mar 7, 2022

Conversation

mashikag
Copy link
Contributor

@mashikag mashikag commented Feb 25, 2022 •
edited

Description

This commit ensures that core blocks are always displayed first, ahead of, non-core blocks in the ScreenBlockList component of GlobalStylesUI.

Currently there is an issue that when non-core blocks are registered they are displayed first, ahead of, non-core blocks. This PR is fixing this issue. See the follow-up, before and after, screenshots for reference.

The issue being fixed was discovered and raised here.

Testing Instructions

  1. Install Jetpack plugin on your local gutenberg dev env site. This does not have to be necessarily Jetpack. Any plugin that registers non-core blocks should do.

  2. Activate the Jetpack plugin. This can be done from Plugins -> Installed Plugins.

  3. Open site editor, via Appearance -> Editor, or by navigating to http://yoursite.com/wp-admin/themes.php?page=gutenberg-edit-site

  4. In the top right of the editor, click at Styles icon. This should open up Styles sidebar on the right hand side of your
    editor's screen.
    Screenshot 2022-02-25 at 12 07 35

  5. Expand the Blocks section in the Styles sidebar, by clicking at the section's sub-header.

  6. Notice how the Contact info and Markdown non-core blocks are displayed ahead of the core blocks when there is no fix applied, and vice-versa when the fix is applied.

Screenshots

Before

Notice how the Business hours to Tiled Gallery blocks, which are non-core, are displayed in front of the core blocks.
before-blocks-list-ordering-fix

After

Now notice how the Business hours to Tiled Gallery blocks, which are non-core, are displayed after core blocks.
after-block-list-ordering-fix

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@github-actions
Copy link

@github-actions github-actions bot commented Feb 25, 2022

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @mashikag! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@mashikag mashikag mentioned this pull request Feb 25, 2022
Closed
@Mamaduka Mamaduka requested review from youknowriad and ntsekouras Feb 25, 2022
ntsekouras
ntsekouras reviewed Feb 25, 2022
View changes
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Thanks for the PR @mashikag!

I've left a comment and also it's a good opportunity to use getBlockTypes selector from blocks store.

packages/edit-site/src/components/global-styles/screen-block-list.js Outdated Show resolved Hide resolved
@mashikag
Copy link
Contributor Author

@mashikag mashikag commented Feb 25, 2022

@ntsekouras Thanks for your review and the suggestion. I will fix this up later this evening. 🙇‍♂️

@mashikag mashikag force-pushed the fix/blocks-order-in-screen-block-list-of-global-styles branch 4 times, most recently from 5167acb to 2036446 Compare Mar 7, 2022
@mashikag
Copy link
Contributor Author

@mashikag mashikag commented Mar 7, 2022

BTW @ntsekouras after I run npm install the package-lock.json changes too. It is of course not cause by my changes but probably someone who did update some project's dependencies and did not include the lock file changes. Should I include the lock file in the PR?

@ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Mar 7, 2022

BTW @ntsekouras after I run npm install the package-lock.json changes too. It is of course not cause by my changes but probably someone who did update some project's dependencies and did not include the lock file changes. Should I include the lock file in the PR?

No you shouldn't. I guess you're talking about the are-we-there-yet addition, that is sometimes caused due to some npm version mismatch between local dev env and expected version from GB.

ntsekouras
ntsekouras approved these changes Mar 7, 2022
View changes
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Besides the small nit about the declaration of the new hook outside return, this looks good!

Thanks so much for the PR @mashikag ! 💯

@mashikag mashikag force-pushed the fix/blocks-order-in-screen-block-list-of-global-styles branch from 2036446 to 56a7fd1 Compare Mar 7, 2022
@mashikag
Fix ordering of blocks in the ScreenBlockList of GlobalStylesUI compo…
b3cd261 
…nent

This commit ensures that core blocks are always displayed first, ahead of, non-core blocks in the ScreenBlockList component of GlobalStylesUI.
@mashikag mashikag force-pushed the fix/blocks-order-in-screen-block-list-of-global-styles branch from 56a7fd1 to b3cd261 Compare Mar 7, 2022
@mashikag
Copy link
Contributor Author

@mashikag mashikag commented Mar 7, 2022

@ntsekouras No problem! Thanks for your guidance. :) I updated the PR once again to align with your most recent suggestions. 👍

@ntsekouras ntsekouras merged commit f4a52b6 into WordPress:trunk Mar 7, 2022
22 checks passed
@github-actions
Copy link

@github-actions github-actions bot commented Mar 7, 2022

Congratulations on your first merged pull request, @mashikag! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 12.8 milestone Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntsekouras ntsekouras

@youknowriad youknowriad

Assignees
No one assigned
Labels
First-time Contributor Global Styles
Projects
None yet
Milestone
Gutenberg 12.8
Linked issues

Successfully merging this pull request may close these issues.

None yet
2 participants
@mashikag @ntsekouras