Update: Use single block editor in the widgets screen #22140
Merged
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
jorgefilipecosta
added
[Type] Enhancement
jorgefilipecosta
requested review from
ajitbohra,
ellatrix,
Soean,
talldan and
youknowriad
as code owners
|
Size Change: -255 B (0%) Total Size: 835 kB
ℹ️ View Unchanged
|
| } ); | ||
| } ) | ||
| ); | ||
| }, [ areas ] ); |
There was a problem hiding this comment.
Should blocks also be listed in the dependency array?
There was a problem hiding this comment.
Nice catch @ZebulanStanphill, I think we should list it 👍
jorgefilipecosta
force-pushed
the
try/use-single-block-editor-for-widgets
branch
2 times, most recently
from
0f9b2a2 to
c4ed88a
Compare
jorgefilipecosta
force-pushed
the
try/use-single-block-editor-for-widgets
branch
2 times, most recently
from
48da84a to
0ca1b33
Compare
jorgefilipecosta
force-pushed
the
try/use-single-block-editor-for-widgets
branch
from
0ca1b33 to
a1c15cb
Compare
jorgefilipecosta
force-pushed
the
try/use-single-block-editor-for-widgets
branch
from
a1c15cb to
8746f6d
Compare
|
This PR is a dependency for some work we are doing on the widgets screen. I'm going to merge the PR so the work can advance more easily. In case there is any comment regarding a change done here please leave a comment and I will gladly address it as a follow-up PR. |
noahtallen
reviewed
May 18, 2020
There was a problem hiding this comment.
Hm, nothing stood out to me as problematic after seeing #22447.
| ); | ||
| return ( | ||
| <InnerBlocks | ||
| __experimentalBlocks={ blocks } |
There was a problem hiding this comment.
This should be changed to value now that #21368 has been merged
6 tasks
noahtallen
added a commit
that referenced
this pull request
May 19, 2020
In #22140, a change was made to the inserter to insert the block at the current selection. Since the e2e test had selected test inside a nested template part, the block was being inserted there, though it was supposed to be inserted at the end of the document. Before that PR, the behavior was to insert it at the end of the document, and not at the current selection. The fix is to select the document in the block breadcrumb before adding the block. This clears any selection so that adding the block block dirties the root entity and not a nested entity.
6 tasks
talldan
reviewed
Jul 3, 2020
| } = select( 'core/block-editor' ); | ||
| const { getBlockVariations } = select( 'core/blocks' ); | ||
|
|
||
| rootClientId = | ||
| rootClientId || getBlockRootClientId( clientId ) || undefined; | ||
| rootClientId || | ||
| getBlockRootClientId( clientId || getBlockSelectionEnd() ) || |
There was a problem hiding this comment.
#23263 proposes to revert this particular change, as it caused the default root appender to insert blocks wherever selection is.
There was a problem hiding this comment.
Seems logical @talldan the widget screen should inject the rootClientId as a prop of the inserter instead.
6 tasks
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
All the other screens, we have used a single block editor. The widget screen used multiple block editors. The fact multiple block editors were used always presented additional complexity, e.g., we needed a system external to the block editor that tracks which widget area is active, and then we needed to use slots with bubblesvirtually to show UI pieces like the inserter bock inspector for the currently selected widget area.
On each feature we want to include, we would need to have some kind of custom implementation, e.g.: adding a block navigator for all the widget areas would evolve some complex slot fills to have it working.
I was trying to add the block navigator and tried to move them to a single block editor instead. The move proved to be very simple; we already have all the pieces to save the inner blocks of a block in a custom location, and it worked well. I ended up spending most of the time of this PR, fixing styling issues and handling things like events to unselect blocks, etc.
But things seem to be working well, and this PR ends up adding a new feature to the widget screen while removing 100+ lines of code.
The change also as some a11y advantages e.g., now we can use block editor select mode (ESC keypress) to navigate around widget areas, and tabbing is similar to other screens while previously tabbing was inconsistent and different from the other screens.
I think applying this change makes the widget screen even simpler and will make the future maintenance of the screen much easier is it now works like the other screens.
How has this been tested?
I verified the widget screen still worked well, allowed saving, etc.
I verified I could use the block navigator on the widget screen.