-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Navigation: Fix sidebar title #52167
Conversation
Size Change: -4 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
Flaky tests detected in d3a9b8c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5455027662
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
2e2f9b4
to
d3a9b8c
Compare
This introduces the same error you fixed recently - #51589. I can confirm with the Console theme but not with TT3; not sure what the difference is. I noticed while checking the e2e test failure for Navigation block tests.
|
This reverts commit ea6b5a1.
I have a fix up for this here. @scruffian |
We shouldn't have this variance. Makes me think something is wrong in one of the responses from REST based on |
I can't replicate the problem I was having now... |
@getdave, @scruffian, did you get a chance to narrow down the cause of the problem? I saw this report #50557 yesterday, and I think it's related to this behavior. |
I haven't had time to narrow this down. To do this we need to:
I think it's likely to do with the I believe instead of accessing title directly we should call |
Thanks, Dave! |
See and gutenberg/packages/core-data/src/entities.js Lines 232 to 237 in f91b4fb
|
How about this? #52635 |
What?
This updates the title in the Navigation section of Patterns to use the name of the navigation itself.
Why?
We should show the actual name of the navigation.
How?
Just reference the title prop directly.
Testing Instructions