-
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
Widget Editor: Refactor header using the DocumentTools component from the editor package #57660
Conversation
Size Change: -714 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
… the editor package
ada9662
to
778da1e
Compare
useEffect( () => { | ||
if ( isInserterOpened && ! isLastSelectedWidgetAreaOpen ) { | ||
// Select the last selected block if hasn't already. | ||
selectBlock( widgetAreaClientId ); | ||
// Open the last selected widget area when opening the inserter. | ||
setIsWidgetAreaOpen( widgetAreaClientId, true ); | ||
} | ||
}, [ | ||
widgetAreaClientId, | ||
isInserterOpened, | ||
isLastSelectedWidgetAreaOpen, | ||
selectBlock, | ||
setIsWidgetAreaOpen, | ||
] ); |
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.
IIRC there was a reason @adamziel or @noisysocks might know more. |
I also checked whether the widgets editor should use the editor package or not and decided that for now it should not. The editor package components rely on the fact that the editor is about editing a given entity and this is provided as a prop to the EditorProvider component which is responsible for initializing the store properly. As it stands, this abstraction is not applicable to the widgets editor because it has no root entity to be edited. While it's true that some components can work like shown here, this is only due to the fact that we have global stores in WP but this is not a given. So I think we should not try to reuse the package and the components for the moment. That said if we think about it, the widget areas are not different than template parts conceptually, so in a longer timeframe, I see no reason for not moving these to the site editor and just editing them exactly like template parts |
@talldan @youknowriad Thank you for the advice. Now, I would like to close this PR and consider adjusting the style while keeping DocumentTools in the Widget Editor 👍 |
Related to #57214
What?
This PR refactors the header in the Widget Editor using the
DocumentTools
component from the@wordpress/editor
package instead of the proprietaryDocumentTools
component.At the same time, we want the "Show button text labels" setting to be applied to the header area as well.
Before
Document tool buttons are 36px in size unlike other editors. Additionally, the focus outline is cut off.
Mobile Layout:
After
The document tools button size is 32 pixels, similar to other editors. Also, the focus outline is not cut.
The Widget Editor doesn't have the preferences modal, but when you turn on "Show button text labels" in the Post Editor or the Site Editor, the buttons in the header display text instead of icons.
Mobile Layout:
Why?
The Widget Editor header has a different UI compared to other editors. The approach taken in #57214 to unify
DocumentTools
component should also be applicable to the Widget Editor.How?
DocumentTools
setIsInserterOpened
andsetIsListViewOpened
actionsTesting Instructions