-
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
Global styles: improve Navigator usage in typography panel #65942
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@@ -53,10 +53,10 @@ function FontSize() { | |||
|
|||
// Navigate to the font sizes list if the font size is not available. | |||
useEffect( () => { | |||
if ( ! fontSize ) { | |||
goTo( '/typography/font-sizes/', { isBack: true } ); |
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.
Using goTo
here was hiding a bug, where goTo
was being called twice. The fix is to also check for the slug
, which then makes the code only run once
@@ -302,7 +302,7 @@ function GlobalStylesUI() { | |||
<ScreenTypography /> | |||
</GlobalStylesNavigationScreen> | |||
|
|||
<GlobalStylesNavigationScreen path="/typography/font-sizes/"> | |||
<GlobalStylesNavigationScreen path="/typography/font-sizes"> |
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.
The trailing slash in the path
was preventing Navigator
from considering /typography/font-sizes/
as a valid parent screen to /typography/font-sizes/:origin/:slug
when navigating back. Removing the trailing slack seems to fix the behavior
Size Change: -12 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
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.
Tests well and code LGTM 👍
Thanks for the fixes @ciampo!
8ae4621
to
28ce403
Compare
…#65942) * Global styles: remove trailing slack from font sizes screen path * Remove unncessary goTo * Use goBack instead of goTo, add extra slug check to avoid double goBack call --- Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]>
What?
Part of #65794
Follow up to #65791
This PR improves the usage of the
Navigator
component in the Font Size screen in the Global StylesWhy?
There are a few areas in the font size presets screen where
Navigator
is not being used as expected:goTo
is being used instead ofgoBack
, and it's there to "hide" a bug wheregoTo
is being called twice inside auseEffect
goTo
is being called unnecessarily when the user clicks on the "back" button, and is hiding a bug caused by a wrongly formatted screen pathHow?
slug
, so that the check inside theuseEffect
only passes once and thus allows us to usegoBack
instead ofgoTo
path
, which allowsNavigator
to navigate back to the correct screen without the need for the extragoTo
Testing Instructions
Screenshots or screencast
Kapture.2024-10-08.at.12.21.01.mp4