Conversation
|
Here's a demo of current progress:
|
|
|
|
I have noticed that none of the values of the panels are working, as in there are not saving to the elements state or updating the display. Is this just me? |
|
@spacedmonkey Not just you! Looks like it's because the Edit: confirmed that moving that prop fixes it. |
I didn't notice. Will fix - it's fairly simple. |
Works for me - overflow is set to auto. However the whole editor is scrolling weirdly because it's sized wrong overall, that might interfere. |
spacedmonkey
left a comment
There was a problem hiding this comment.
@barklund Just noticed that titles are clickable. Number 1, this felt wrong as seems like it should be. It will also be a weird UX for WP users. See this video. https://youtu.be/CRR6ac68Q5M
| ); | ||
|
|
||
| return ( | ||
| <Header isPrimary={ isPrimary }> |
There was a problem hiding this comment.
Adding the necessary aria- attributes sounds easy enough for now 👍
There was a problem hiding this comment.
I've more or less done what the W3 guide recommends. I don't know how important it is for IDs to be semantic (as in: is 'panel_123' as good as 'panel_size' as it's never read out loud, just linking items), but I've added that now too.
| const { state: { isCollapsed, height } } = useContext( panelContext ); | ||
|
|
||
| if ( isCollapsed ) { | ||
| return null; |
There was a problem hiding this comment.
I was wonder, if this doesn't render, does it have accessibility impacts here?
There was a problem hiding this comment.
This shouldn't be a problem because users would never be able to tab into a non-expanded area.
Gutenberg also doesn't have any special handling in this regard.
There was a problem hiding this comment.
Hadn't thought about this. Will leave the comment open for discussion. I haven't seen any recommendations in this regard.
There was a problem hiding this comment.
The W3 example add hidden attribute and hides with CSS. Replicated here now.
|
|
| }, [ isDragging, handleMouseUp, handleMouseMove, handleMouseDown ] ); | ||
|
|
||
| return ( | ||
| <Handle ref={ handle }> |
There was a problem hiding this comment.
What accessibility could we add here? CC @pbakaus as he has done a lot of accessibility with drag and drop.
There was a problem hiding this comment.
My thinking:
- Keyboard shortcuts to go toggle resize mode and do resizing. Example:
Space to toggle resize mode, Up and Down to resize - Screenreader messages (
aria-live) to announce state changes
There was a problem hiding this comment.
So, I've done it as follows:
amp-wp/assets/src/edit-story/panels/panel/handle.js
Lines 46 to 56 in cebef75
Plus it has a keyboard handler reacting to up and down arrow presses, that then change the height up and down.
This is partly informed by this question, but I'm using role="slider" actively going against one of the recommendations there, as it allows me to announce the range and current value with other attributes. Thus I don't think it needs aria-live, which could be a bit weird here.
I was considering whether to use aria-valuetext to announce the height in relative terms - e.g. "Panel is 55% of maximum height" or something similar. That might be a bit too much?
I've also included the information about what the button does as actual textual content of the button (but hidden it for browser display with text-indent). I tihnk this is recommended over using aria-label.
| height: height === null ? 'auto' : `${ height }px`, | ||
| }; | ||
|
|
||
| return ( |
The design seems to indicate that it's intended behavior, but to be sure on the way forward we'd need to get confirmation from UX.
I cannot exactly reproduce this. It stops here:
However, as I continue dragging, the height increases, causing some weird behavior in the canvas area (i.e. the canvas and carousel move down due to lots of additional space):
Seems some kind of limit needs to be respected here. |
…wp into feature/3853-layer-panel
I am seeing this in Chrome and Firefox. See this video. |
Note that this does not reset on resize. useResizeObserver should be used once merged in to the same branch as this code.
I've now added sensible limits in b03fc39. |
| @@ -0,0 +1,11 @@ | |||
| /** | |||
There was a problem hiding this comment.
Shouldn't this be with other buttons in edit-story\components\button
Summary
This PR includes some general changes to design and architecture for panels necessary for future work.
This PR will not address:
Partially fixes #3853.
Checklist