[edit-widgets] Improve the UX of wp_inactive_widgets on the experimental screen#24790
Merged
Conversation
adamziel
added
[Feature] Widgets Screen
adamziel
requested review from
TimothyBJacobs,
spacedmonkey and
youknowriad
as code owners
20 tasks
|
Size Change: +29 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
draganescu
reviewed
Aug 25, 2020
| $sidebar['status'] = 'inactive'; | ||
| } | ||
|
|
||
| if ( 'wp_inactive_widgets' === $sidebar['id'] && empty( $sidebar['name'] ) ) { |
Contributor
There was a problem hiding this comment.
Maybe make 'wp_inactive_widgets' === $sidebar['id'] && empty( $sidebar['name'] ) into another is variable $is_inactive_widgets_sidebar just to make it like $is_registered_sidebar ... not important.
Contributor
Author
There was a problem hiding this comment.
It would have to be just $is_inactive_widgets_sidebar = 'wp_inactive_widgets' === $sidebar['id'], the name may or may not be empty. It would be a tad more readable, but since that line appears only once per method and there is no duplication, the benefits are arguable - I'd say it's a matter of taste.
Member
|
Would be nice to have a test on this. |
noisysocks
reviewed
Aug 26, 2020
Member
noisysocks
left a comment
noisysocks
left a comment
There was a problem hiding this comment.
This works well. I agree that we should add a PHPUnit test. Can we please also move the inactive widgets to the bottom of the new screen and have it collapsed by default?
adamziel
changed the title
adamziel
force-pushed
the
update/remove-inactive-widgets-area
branch
from
9f13149 to
3362413
Compare
adamziel
force-pushed
the
update/remove-inactive-widgets-area
branch
from
3362413 to
bb75163
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
At the moment, there is an empty area at the top of the Block Areas screen which has no title. It is the inactive widgets area. Widgets in that area are not being correctly stored/returned by the API and they are lost after leaving the page. After applying this PR, the interaction in question works properly:
Before:
After:
Related comment: #24290 (review)
How has this been tested?
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: