Inserter: Clarify that when the Inserter is open clicking the + button in the top bar will close it again #29759
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
jameskoster
added
[Focus] Accessibility (a11y)
|
Size Change: +11.7 kB (+1%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
|
Hey James. I am comparing directly to the Settings gear icon. How we open and close Settings today. Open-close-settings-gear-icon.mp4Comparing to opening and closing Settings.
I am wondering if the tooltip should stay as it is today. Similar to Settings has the tooltip "Settings". |
|
This makes sense to me. I don't love losing the experience of my content bouncing around when I'm using the Inserter. I would rather it remain open. The only slightly strange thing to me is the + animating to the close button. I'm not sure that's necessary as it deviates from the other panel buttons' open state. I might change the background to black and leave the + icon as is. |
|
Turning the plus into a cross was immediately what I thought of when reading this issue title, so that's a +1 from me. |
jasmussen
approved these changes
Mar 17, 2021
| label={ | ||
| isInserterOpened | ||
| ? _x( | ||
| 'Close block inserter', |
There was a problem hiding this comment.
Could we replace this with a single "Toggle block inserter" instead of the ternery? That way the label doesn't need to change, which I suspect might be better for screen readers.
|
|
||
| &.is-pressed { | ||
| svg { | ||
| transform: rotate(45deg); |
There was a problem hiding this comment.
If we go and keep this, it might be nice to update the Figma "small x" icon to be more or less identical to the plus as rotated.
There was a problem hiding this comment.
Agreed, let's assess in a follow up.
There was a problem hiding this comment.
I checked, it's close but no cigar 😅
There was a problem hiding this comment.
Hah, I bet that when the pixels antialias together the difference is barely perceptible.
Of course that won't be good enough for you or me, because we'll know! But I'm happy to push the vectors once we get to it 😄
| ) | ||
| : _x( | ||
| 'Add block', | ||
| 'Generic label for block inserter button' |
There was a problem hiding this comment.
Same as before, it seems simpler and safer to have a single "toggle" label.
| @@ -81,6 +81,16 @@ body.is-navigation-sidebar-open { | |||
| width: $grid-unit-40; | |||
| height: $grid-unit-40; | |||
| padding: 0; | |||
|
|
|||
| svg { | |||
| transition: transform cubic-bezier(0.165, 0.84, 0.44, 1) 0.2s; | |||
There was a problem hiding this comment.
Same as before, include the mixin.
|
You're getting this test failure: So it looks like you need to update that test (can probably just search the codebase for |
|
Let's give that a try 🤞 |
|
This is what I see: Add-block-Inserter-X.mp4It looks nice with the switch to the X. The panel does not stay open when clicking into the Gutenberg layout area. |
|
Hey James. @jameskoster |
This is not intended. When you click outside the inserter, it should close, same as if you press Escape, because the inserter traps focus like a modal. The inserter should stay open when you click a block to insert it, or drag and drop it. |
|
Hi Joen Understand. Here is a comment in relation to keeping the Add block panel open as if it was a sidebar. As the Add block Inserter looks like a sidebar. #22871 (comment) |
|
Yep, I don't disagree with the basic premise, and it's the reason why the List View stays permanently open. It is, however, a delicate balance that strives to ensure powerful access to inserting block and navigating the canvas for both mouse and screen reader users. It can be revisited, of course, but it's very non-trivial, and probably not something that should block this PR. |
| @@ -41,7 +41,7 @@ export async function closeGlobalBlockInserter() { | |||
| async function isGlobalInserterOpen() { | |||
| return await page.evaluate( () => { | |||
| return !! document.querySelector( | |||
| '.edit-post-header [aria-label="Add block"].is-pressed, .edit-site-header [aria-label="Add block"].is-pressed' | |||
| '.edit-post-header [aria-label="Toggle block inserter"].is-pressed, .edit-site-header [aria-label="Add block"].is-pressed' | |||
There was a problem hiding this comment.
There is a second instance of "Add block" there. (Used for site editor)
| @@ -50,7 +50,7 @@ async function isGlobalInserterOpen() { | |||
| */ | |||
| export async function toggleGlobalBlockInserter() { | |||
| await page.click( | |||
| '.edit-post-header [aria-label="Add block"], .edit-site-header [aria-label="Add block"]' | |||
| '.edit-post-header [aria-label="Toggle block inserter"], .edit-site-header [aria-label="Add block"]' | |||
There was a problem hiding this comment.
There is a second instance of "Add block" there. (Used for site editor)
david-szabo97
approved these changes
Mar 26, 2021
|
@david-szabo97 but it is un-mergeable until all tests pass 😬 |
hedgefield
approved these changes
Mar 29, 2021
youknowriad
reviewed
Mar 29, 2021
| @@ -41,7 +41,7 @@ export async function closeGlobalBlockInserter() { | |||
| async function isGlobalInserterOpen() { | |||
| return await page.evaluate( () => { | |||
| return !! document.querySelector( | |||
| '.edit-post-header [aria-label="Toggle block inserter"].is-pressed, .edit-site-header [aria-label="Toggle block inserter"].is-pressed' | |||
| '.edit-post-header [aria-label="Add block"].is-pressed, .edit-site-header [aria-label="Add block"].is-pressed, .edit-post-header [aria-label="Toggle block inserter"].is-pressed, .edit-site-header [aria-label="Toggle block inserter"].is-pressed' | |||
There was a problem hiding this comment.
Can we add a comment explaining why we need both, so future contributors can make good decisions about when to remove.
gziolo
changed the title
paaljoachim
added
[Type] Enhancement
7 tasks
7 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.
Trying out @paaljoachim's idea from #22871 to see how it feels. Video demo in both the post editor and the site editor:
inserter.mp4
Suggestions welcome for the context and tooltip label on the button when in close orientation.