Nav block: link text color inheritance fixes and tests #51710
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MaggieCabrera
changed the title
|
Size Change: +5.56 kB (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in c693611. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5534144204
|
getdave
reviewed
Jun 20, 2023
getdave
reviewed
Jun 21, 2023
getdave
reviewed
Jun 22, 2023
MaggieCabrera
force-pushed
the
fix-nav-colors
branch
2 times, most recently
from
c621dda to
8273dba
Compare
MaggieCabrera
force-pushed
the
fix-nav-colors
branch
from
40f0454 to
37757d1
Compare
MaggieCabrera
requested review from
kevin940726,
Mamaduka and
tellthemachines
as code owners
MaggieCabrera
added
[Block] Navigation
This comment was marked as outdated.
This comment was marked as outdated.
getdave
reviewed
Jun 30, 2023
getdave
reviewed
Jun 30, 2023
getdave
reviewed
Jun 30, 2023
getdave
reviewed
Jun 30, 2023
|
We might want to consider splitting the Navigation tests into smaller/logical chunks to leverage sharding better. |
Mmmh, I'm not sure if I'm missing something, but I think we already do that? This particular PR groups it's tests on their own, and we've been doing that in other instances. Or do you mean, moving the blocks from the general "Navigation block" section like the interactivity tests are doing? If that's the case, I can make the change for this PR tests and then follow up for the rest on another PR. |
|
@MaggieCabrera, sorry, I mean splitting tests into separate files. We don't have to do it here, but something to consider as the Navigation block tests grow. |
Yeah, that probably makes sense |
MaggieCabrera
force-pushed
the
fix-nav-colors
branch
2 times, most recently
from
1ace2bd to
ecb54df
Compare
I'm right there with you. I tried to comment as thoroughly as possible. There's probably some improvements we can make like making the colors into variables like I did for the biggest test (where the RGB codes are repeated multiple times and become less and less readable). Happy to hear any other improvements anyone can suggest. |
westonruter
added a commit
that referenced
this pull request
Jul 13, 2023
…dd/defer-script-loading-strategy * 'trunk' of https://github.com/WordPress/gutenberg: (24 commits) Add filter to turn off Interactivity API for a block (#52579) Search: Remove unnecessary useEffect (#52604) Navigation: Simplify the useSelect for useNavigationMenus (#51977) Item: Unify focus style and add default font styles (#52495) Update Changelog for 16.2.1 Bump plugin version to 16.2.1 Avoid passing undefined `selectedBlockClientId` in `BlockActionsMenu` (#52595) Cover Block: Fix block deprecation when fixed background is enabled (#51612) Nav block: link text color inheritance fixes and tests (#51710) Stabilize defaultBlock, directInsert API's and getDirectInsertBlock selector (#52083) Fix console warning by improving error handling in Nav block classic menu conversion (#52591) Fix: Remove link action of Link UI for draft pages created from Nav block does not correctly remove link. (#52415) Lodash: Remove remaining `_.get()` from block editor and deprecate (#52561) Fix importing classic menus (#52573) ResizableFrame: Make keyboard accessible (#52443) Site Editor: Fix navigation menu sidebar actions order and label (#52592) correct a typo: sapce -> space (#52578) Avoid errors in Dimension visualizers when switching between iframed and non-iframed editors (#52588) Patterns: Add client side pagination to patterns list (#52538) Site Editor: Make sidebar back button go *back* instead of *up* if possible (#52456) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Closes #51575
🤖 Generated by Copilot at 27b5873
This pull request refactors the color and layout styles of the navigation block and its items. It fixes a bug with the color inheritance, removes the
__experimentalStyleproperty fromblock.json, and adds tests to cover possible regressions in the future.Why?
The color settings for the navigation block have had multiple regressions over time. Instead of simply solving #51575 I've decided to add test coverage to this feature.
Related PRs:
#41898
#31149
#44578
How?
🤖 Generated by Copilot at 27b5873
__experimentalStyleproperty from the navigation block'sblock.jsonfile to avoid conflicts with the global styles system and the link color feature (link):wherepseudo-class and lower the specificity to allow theme overrides (link)Testing Instructions
Run
npm run test:e2e:playwright -- -g "Navigation colors"Ideally, the tests passing are all the testing this PR requires, but we might be missing edge cases that are not covered by them, so the best testing this PR can have is check that the tests make sense and try to find gaps that we might be missing.