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

Navigation Link: Remove color generation code #48927

Merged
merged 1 commit into from
Mar 10, 2023
Merged

Navigation Link: Remove color generation code #48927

merged 1 commit into from
Mar 10, 2023

Conversation

scruffian
Copy link
Contributor

@scruffian scruffian commented Mar 8, 2023 •
edited
Loading

What?

This fixes a bug that was introduced in #48219. The colors of the submenu should override the normal block colors inside the submenu.

Why?

The settings to customize the colors of the block should apply to top level links, the settings for the submenu/overlay should apply inside the block.

How?

There's no need to generate colors for navigation links themselves, they can just inherit from the parent they are in, so we can just remove this code.

Testing Instructions

  1. Add a navigation block with several links inside it
  2. Add some submenus too
  3. Customize the colors of the block and the submenu
  4. Check that the block colors are applied to the top level items and the submenu colors are applied to items in the submenu,

Screenshots or screencast

Before (frontend)
Screenshot 2023-03-08 at 13 48 38

After (frontend)
Screenshot 2023-03-08 at 13 49 16

Editor (unch
Screenshot 2023-03-08 at 13 48 56
anged)

@scruffian scruffian self-assigned this Mar 8, 2023
@scruffian scruffian added [Block] Navigation Affects the Navigation Block [Block] Navigation Link Affects the Navigation Link Block labels Mar 8, 2023
@ntsekouras
Copy link
Contributor

We'd probably need to backport this to 6.2, right?

@scruffian scruffian mentioned this pull request Mar 8, 2023
Closed
MaggieCabrera
MaggieCabrera approved these changes Mar 10, 2023
View reviewed changes
Copy link
Contributor

@MaggieCabrera MaggieCabrera left a comment

Choose a reason for hiding this comment

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

This is working as intended and a site with previously selected colors still work nicely after this PR

@draganescu draganescu merged commit 4b283a4 into trunk Mar 10, 2023
@draganescu draganescu deleted the fix/navigation-link-colors branch March 10, 2023 10:23
@github-actions github-actions bot added this to the Gutenberg 15.4 milestone Mar 10, 2023
@ntsekouras ntsekouras added [Type] Bug An existing feature does not function as intended Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Mar 10, 2023
Mamaduka pushed a commit that referenced this pull request Mar 14, 2023
@scruffian @Mamaduka
Navigation Link: Remove color generation code (#48927)
a1aafaa
@Mamaduka
Copy link
Member

I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: a1aafaa

@Mamaduka Mamaduka removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Mar 14, 2023
@Mamaduka Mamaduka mentioned this pull request Mar 14, 2023
Closed
@hellofromtonya
Copy link
Contributor

@Mamaduka when was block_core_navigation_link_build_css_colors() introduced into WordPress Core? Was it during the 6.2 cycle?

@Mamaduka
Copy link
Member

Mamaduka commented Mar 14, 2023 •
edited
Loading

@hellofromtonya, no, the function was introduced a few versions earlier. But the bug mentioned in the PR description was introduced in beta 5.

Edit: The function was introduced in #21075.

@hellofromtonya
Copy link
Contributor

Thanks @Mamaduka. A hard removal of the PHP function breaks BC (Backwards Compatibility). So instead of removing it,

  • If no longer needed, then it needs to be deprecated instead, i.e. using _deprecated_function(). This will alert extenders to modify their code to not invoke this function.
  • Or keep the function but fix it.

@Mamaduka
Copy link
Member

Re: block_core_navigation_link_build_css_colors:

  • Let’s keep the function in 6.2 and only apply the fix.
  • Deprecate it in the next major version.

@Mamaduka Mamaduka mentioned this pull request Mar 14, 2023
Merged
@Mamaduka
Copy link
Member

Here's the PR - #49064.

markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Mar 14, 2023
@hellofromtonya
Editor: Update @WordPress packages for 6.2 RC2.
48deabc 
Updates the `@wordpress` packages to include the following changes:

- [Regression] Don't offer Classic block as a recovery action when not registered [WordPress/gutenberg#49051 Gutenberg PR 49051] ✅ 
- [i18n Change] Fix typo in the media-categories component [WordPress/gutenberg#49047 Gutenberg PR 49047] ✅
- Hide navigation screen in site editor [WordPress/gutenberg#49043 Gutenberg PR 49043] ✅ 
 [https://make.wordpress.org/core/2023/03/14/fyi-navigation-section-of-new-site-editor-experienced-removed-for-6-2-rc-2/ Feature is not ready for 6.2]
- [i18n Change] Site editor: Fix non-us spelling in sidebar [WordPress/gutenberg#48976 Gutenberg PR 48976] ✅ See Trac #57895.
- [Regression] Site Editor: Fix lingering insertion point within template parts [WordPress/gutenberg#48913 Gutenberg PR 48913] ✅ > Regression introduced in 6.2 cycle.
- Navigation Link: Remove color generation code [WordPress/gutenberg#48927 Gutenberg PR 48927] and [WordPress/gutenberg#49064 Gutenberg PR 49064] ✅
- Fix settings tab active state border in block inspector [WordPress/gutenberg#48945 Gutenberg PR 48945] ✅
- Fix text alignment in the Site Editor sidebar  [WordPress/gutenberg#48959 Gutenberg PR 48959] ✅ Making template descriptions more prominent is part of 6.2.

References:
* [WordPress/gutenberg@f22a3cb Packages x.3.9 publish commit]
* [WordPress/gutenberg@356298f Packages x.3.10 publish commit]

Follow-up to [55496].

Props mamaduka, tobifjellner, davidbaumwald, costdev, audrasjb, hellofromTonya.
See #57471.
Fixes #57895.
Built from https://develop.svn.wordpress.org/trunk@55542


git-svn-id: http://core.svn.wordpress.org/trunk@55054 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this pull request Mar 14, 2023
@hellofromtonya
Editor: Update @WordPress packages for 6.2 RC2.
f71da79 
Updates the `@wordpress` packages to include the following changes:

- [Regression] Don't offer Classic block as a recovery action when not registered [WordPress/gutenberg#49051 Gutenberg PR 49051] ✅ 
- [i18n Change] Fix typo in the media-categories component [WordPress/gutenberg#49047 Gutenberg PR 49047] ✅
- Hide navigation screen in site editor [WordPress/gutenberg#49043 Gutenberg PR 49043] ✅ 
 [https://make.wordpress.org/core/2023/03/14/fyi-navigation-section-of-new-site-editor-experienced-removed-for-6-2-rc-2/ Feature is not ready for 6.2]
- [i18n Change] Site editor: Fix non-us spelling in sidebar [WordPress/gutenberg#48976 Gutenberg PR 48976] ✅ See Trac #57895.
- [Regression] Site Editor: Fix lingering insertion point within template parts [WordPress/gutenberg#48913 Gutenberg PR 48913] ✅ > Regression introduced in 6.2 cycle.
- Navigation Link: Remove color generation code [WordPress/gutenberg#48927 Gutenberg PR 48927] and [WordPress/gutenberg#49064 Gutenberg PR 49064] ✅
- Fix settings tab active state border in block inspector [WordPress/gutenberg#48945 Gutenberg PR 48945] ✅
- Fix text alignment in the Site Editor sidebar  [WordPress/gutenberg#48959 Gutenberg PR 48959] ✅ Making template descriptions more prominent is part of 6.2.

References:
* [WordPress/gutenberg@f22a3cb Packages x.3.9 publish commit]
* [WordPress/gutenberg@356298f Packages x.3.10 publish commit]

Follow-up to [55496].

Props mamaduka, tobifjellner, davidbaumwald, costdev, audrasjb, hellofromTonya.
See #57471.
Fixes #57895.
Built from https://develop.svn.wordpress.org/trunk@55542


git-svn-id: https://core.svn.wordpress.org/trunk@55054 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Mar 14, 2023
@hellofromtonya
Editor: Update @WordPress packages for 6.2 RC2.
a71a9bc 
Updates the `@wordpress` packages to include the following changes:

- [Regression] Don't offer Classic block as a recovery action when not registered [WordPress/gutenberg#49051 Gutenberg PR 49051] ✅ 
- [i18n Change] Fix typo in the media-categories component [WordPress/gutenberg#49047 Gutenberg PR 49047] ✅
- Hide navigation screen in site editor [WordPress/gutenberg#49043 Gutenberg PR 49043] ✅ 
 [https://make.wordpress.org/core/2023/03/14/fyi-navigation-section-of-new-site-editor-experienced-removed-for-6-2-rc-2/ Feature is not ready for 6.2]
- [i18n Change] Site editor: Fix non-us spelling in sidebar [WordPress/gutenberg#48976 Gutenberg PR 48976] ✅ See Trac #57895.
- [Regression] Site Editor: Fix lingering insertion point within template parts [WordPress/gutenberg#48913 Gutenberg PR 48913] ✅ > Regression introduced in 6.2 cycle.
- Navigation Link: Remove color generation code [WordPress/gutenberg#48927 Gutenberg PR 48927] and [WordPress/gutenberg#49064 Gutenberg PR 49064] ✅
- Fix settings tab active state border in block inspector [WordPress/gutenberg#48945 Gutenberg PR 48945] ✅
- Fix text alignment in the Site Editor sidebar  [WordPress/gutenberg#48959 Gutenberg PR 48959] ✅ Making template descriptions more prominent is part of 6.2.

References:
* [WordPress/gutenberg@f22a3cb Packages x.3.9 publish commit]
* [WordPress/gutenberg@356298f Packages x.3.10 publish commit]

Follow-up to [55496].

Props mamaduka, tobifjellner, davidbaumwald, costdev, audrasjb, hellofromTonya.
Reviewed by SergeyBiryukov.
Merges [55542] to the 6.2 branch.
See #57471.
Fixes #57895.

git-svn-id: https://develop.svn.wordpress.org/branches/6.2@55548 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Mar 14, 2023
@hellofromtonya
Editor: Update @WordPress packages for 6.2 RC2.
7592613 
Updates the `@wordpress` packages to include the following changes:

- [Regression] Don't offer Classic block as a recovery action when not registered [WordPress/gutenberg#49051 Gutenberg PR 49051] ✅ 
- [i18n Change] Fix typo in the media-categories component [WordPress/gutenberg#49047 Gutenberg PR 49047] ✅
- Hide navigation screen in site editor [WordPress/gutenberg#49043 Gutenberg PR 49043] ✅ 
 [https://make.wordpress.org/core/2023/03/14/fyi-navigation-section-of-new-site-editor-experienced-removed-for-6-2-rc-2/ Feature is not ready for 6.2]
- [i18n Change] Site editor: Fix non-us spelling in sidebar [WordPress/gutenberg#48976 Gutenberg PR 48976] ✅ See Trac #57895.
- [Regression] Site Editor: Fix lingering insertion point within template parts [WordPress/gutenberg#48913 Gutenberg PR 48913] ✅ > Regression introduced in 6.2 cycle.
- Navigation Link: Remove color generation code [WordPress/gutenberg#48927 Gutenberg PR 48927] and [WordPress/gutenberg#49064 Gutenberg PR 49064] ✅
- Fix settings tab active state border in block inspector [WordPress/gutenberg#48945 Gutenberg PR 48945] ✅
- Fix text alignment in the Site Editor sidebar  [WordPress/gutenberg#48959 Gutenberg PR 48959] ✅ Making template descriptions more prominent is part of 6.2.

References:
* [WordPress/gutenberg@f22a3cb Packages x.3.9 publish commit]
* [WordPress/gutenberg@356298f Packages x.3.10 publish commit]

Follow-up to [55496].

Props mamaduka, tobifjellner, davidbaumwald, costdev, audrasjb, hellofromTonya.
Reviewed by SergeyBiryukov.
Merges [55542] to the 6.2 branch.
See #57471.
Fixes #57895.
Built from https://develop.svn.wordpress.org/branches/6.2@55548


git-svn-id: http://core.svn.wordpress.org/branches/6.2@55060 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaggieCabrera MaggieCabrera MaggieCabrera approved these changes

@tellthemachines tellthemachines Awaiting requested review from tellthemachines

Assignees

@scruffian scruffian

Labels
[Block] Navigation Link Affects the Navigation Link Block [Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Gutenberg 15.4
Development

Successfully merging this pull request may close these issues.
6 participants
@scruffian @ntsekouras @Mamaduka @hellofromtonya @MaggieCabrera @draganescu