-
Notifications
You must be signed in to change notification settings - Fork 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
Help Center: Display help icon on the view mode of the site editor #93362
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
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.
Thanks for working on this! Can we add some styles to the button when it has the class is-active
(i.e the HC is open)?
Also, I'm not sure if we need to force updating.
// Force to re-render to ensure the sidebar is available. | ||
if ( canvasMode === 'view' ) { | ||
forceUpdate(); | ||
} |
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.
Isn't setShowHelpCenter( false );
sufficient to re-render here?
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.
It doesn't work if it's closed
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.
You're right, I misunderstood this effect.
But the behavior will be different from the search icon and that's why I didn't add the active style. |
Yes, but the search icon is not a toggle, just a button. Whereas the Help Center button is a toggle and it would be consistent with the UX in the editor. Screen.Recording.2024-08-08.at.14.48.13.mov |
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.
Tested in Site Editor and Post editor. All work great.
@@ -15,12 +22,23 @@ function HelpCenterContent() { | |||
href="https://wordpress.com/help" | |||
icon={ <HelpIcon /> } | |||
label="Help" | |||
size="compact" | |||
size={ canvasMode === 'edit' ? 'compact' : undefined } |
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.
I just noticed that it looks like the button is no longer sized to compact
in the post editor's header bar at the top right:
I think in that case it should still be size="compact"
. Is there another way to skip the compact sizing when it's required so that the buttons in the post editor header can line up?
I've logged this in a separate issue over in #93976. I couldn't find an existing issue for it, so apologies if that's a double up!
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.
Oops...the canvas mode seems to be unavailable in the post editor. Will check it later 👍
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.
Yes, the canvas mode only exists conceptually with the site editor for now.
Will check it later 👍
Feel free to add me as a reviewer if you open a PR for it!
Related to #92985
Proposed Changes
help-center-on-editor-sidebar.mov
Why are these changes being made?
Testing Instructions
apps/help-center
.yarn dev --sync
.widgets.wp.com
.Pre-merge Checklist