Nav offcanvas restore click to edit #47608
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
getdave
requested review from
talldan,
SaxonF,
scruffian,
draganescu,
MaggieCabrera and
ajlende
|
Size Change: -120 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in d52b91d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4055108896
|
draganescu
approved these changes
Jan 31, 2023
There was a problem hiding this comment.
I've tested this and now with the animation and the back button all my initial concerns are solved. The UI in the inspector predictably gets replaced with the details of the selected block, there is a small delay that does not flash the new UI in. All good.
The drag and drop functionality works perfectly unaffected by this change. And the code removes a lot of cruft for that edit button.
|
Thanks @draganescu! I'll wait to hear from @SaxonF and @talldan just in case there are any UX or other issues they foresaw (as the originators of the two approaches). Then we can merge. |
|
Sadly this isn't working for me :( this is a fresh install with just some dummy content
|
scruffian
approved these changes
Jan 31, 2023
|
@MaggieCabrera this is known. There used to be a check removed in 421f7ca as hardcoding that block name to avoid this behavior is likely not the way to go about it. At the same time, the normal left hand list view also has this problem. In a page list selecting page list items in the canvas does not work, but it works using the list view. So I think we should treat this as a bug of how locked blocks are handled by list view not a problem of this PR. What do you think? |
yeah, I don't think that should block this PR |
|
@getdave could this PR get rid of the |
scruffian
force-pushed
the
try/nav-offcanvas-restore-click-to-edit
branch
from
0414eb6
to
d40e93e
Compare
scruffian
force-pushed
the
try/nav-offcanvas-restore-click-to-edit
branch
from
d40e93e
to
d52b91d
Compare
|
Looks good, let's bring this in. |
juanmaguitar
added
the
[Type] Regression
|
|
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.
What?
Removes the "edit" button for each node and allows clicking on a node to trigger "edit mode" for that block.
Why?
See rationale in #47023 (comment).
Kudos to @talldan who suggested this originally and received push back 🙇♂️
How?
Ensure only valid blocks are editableWe decided it's better to allow all blocks to be editable and simply imply/hint that certain blocks are not editable via UI affordances. Ultimately if a user wants to check controls on Page List Items they should be able to. It's also better for a11y as it's more consistent to retain same behaviour across all nodes.Testing Instructions
Test not possible to edit Page List Item blocks.No longer needed. See above.Testing Instructions for Keyboard
Screenshots or screencast
Screen.Capture.on.2023-01-31.at.11-38-38.mp4