Add widgets editor more menu #31926
Merged
Add widgets editor more menu #31926
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
talldan
added
[Package] Edit Widgets
talldan
requested review from
adamziel,
draganescu,
kevin940726,
noisysocks and
tellthemachines
as code owners
|
Size Change: +2.55 kB (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
|
Looks great! Can we make it so that toggling the "Contain text cursor" setting doesn't make the menu's width jump around? Kapture.2021-05-19.at.10.40.40.mp4Also, not really related, but on mobile, you have to scroll right to see this new button in the toolbar. Maybe we should hide the "Widgets" title on smaller viewports. Kapture.2021-05-19.at.10.43.11.mp4 |
noisysocks
reviewed
May 19, 2021
| @@ -0,0 +1,66 @@ | |||
| .edit-post-keyboard-shortcut-help-modal { | |||
There was a problem hiding this comment.
This and all the other class names should begin with edit-widgets.
| isModalActive, | ||
| toggleModal, | ||
| } ) { | ||
| useShortcut( 'core/edit-post/keyboard-shortcuts', toggleModal, { |
There was a problem hiding this comment.
Maybe should be edit-widgets?
|
Refactoring into |
|
One other issue: The menu has a higher z-index than the Keyboard Shortcuts overlay. It should appear underneath as it does in the post editor.
|
7 tasks
talldan
force-pushed
the
add/widgets-editor-settings-menu
branch
from
4276707 to
c8e3476
Compare
|
@noisysocks That should all be addressed now. I made #31965 for refactoring to the interface package. |
noisysocks
approved these changes
May 19, 2021
|
Thank you! |
|
Failing tests are also failing in |
|
Why was this merged with failing tests? It's not because trunk is failing that that's ok? |
|
@ellatrix I understand that. The changes here are localised to the widget editors which have separate tests. The failing tests in Similarly, it's frustrating to see that |
|
Looks and works great. Super minor note to say it would be nice to add the "Preferences" subtitle (same style as "View" and "Tools") before the last option. |
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.
Description
Closes #27564
Adds the more menu to the standalone widget editor. Follows the suggestion in #27564 (comment).
The code is a lot of copy and paste, because preferences are implemented in the
edit-postpackage and not reusable across editors. There's a lot of scope for refactor of these preferences (either into their own package, or perhapsinterface?).How has this been tested?
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.jsfiles for terms that need renaming or removal).