GAUD-10312 - No interaction with arrows when divider collapsed - #7334
GAUD-10312 - No interaction with arrows when divider collapsed#7334svanherk wants to merge 4 commits into
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
| if (this.#clickedArrow) return; // Arrows don't support dragging | ||
|
|
||
| const clickedArrow = path.find(el => el.classList?.contains('divider-arrow')); | ||
| if (clickedArrow) { |
There was a problem hiding this comment.
This code moving into the click handler above fixes an eventual bug that would have come up with overlay. It's the same issue the drawer would have had - if we move the divider before the click happens, it'll click what's behind it. I thought this was only an issue with the drawer handle, but it also happens to the arrows in overlay mode, where clicking the overlay closes the drawer.
Even if we don't want that to happen (I need to ask design), it's just better and will likely save us from some other bug in the future.
| this.#clickedHandle = path.some(el => el.classList?.contains('divider-handle')); | ||
| this.#clickedArrow = path.find(el => el.classList?.contains('divider-arrow')); |
There was a problem hiding this comment.
One thing I wanted to call out here - this is a maybe a tad confusing, because one stores a boolean and one stores the actual arrow (since I need it later). I could store the handle to be consistent, but I don't need it. I could use two variables (one to store that the arrow was clicked, and one to store the arrow) but that felt like overkill. Or I could update the naming, but I liked how this flowed 🤷♀️
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Maybe more of a question for design, but if it's not interactive then maybe we should hide the divider arrows when collapsed, since they become inoperable and the slider icon already provides a visual indication of direction when collapsed...? Another question... when I try it out, I notice that the divider arrows only seem to appear when I click on the divider. Is that expected? Should they be displayed if the user hovers over them or the divider? I see that the divider arrows do provide a visual indication of whether they can grow or shrink the side-nav. Although they were originally meant for the pointer non-drag a11y case, I find the visual helpful when using the keyboard with arrow keys, especially when I reach the min/max size. |
Yeah I talked to @glen-bartlett-d2l about both and the outcome of his investigations were:
But I'll poke Glen about this comment.
It's actually the opposite lol - they were originally meant as only the helpful visual indication, and then were re-purposed for the non-drag a11y case after you brought it up! 😄 |
|
I see, thanks. There's definitely a bit going on, now serving the dual purpose. I'll admit I tried to click it while it was collapsed, but maybe a user wouldn't normally do that.
The focus requirement makes it a little harder for mouse/pointer non-drag users, but they'd probably discovered it after they click the slider once or twice. It's not really any harder than the HTML editor resizer - it too needs to be first clicked to toggle into that mode. However in its case, showing those click targets on hover was never an option because the user needs to hover to begin dragging. Anyway, remembering the size hopefully reduces need for resizing as well. |
@svanherk After thinking about this a bit, Dave does have a point about the expand button already showing that directionality when the panel is closed, so showing the other arrow there is a bit misleading (although no more misleading than the one in primary secondary, which looks like something you should be able to click). I don't feel strongly about leaving it there, but if it's easy to change, it might be best to not show it when the panel is closed? |
Yup that's also an easy change - will remove. I think I'll do a separate PR and keep this as is, for history. |
|
Closing in favour of #7341 |
After talking to design, it was decided that the arrows should become inactive when the divider is collapsed, and act simply as direction indicators. The user should use the divider line or the toggle button to open a collapsed divider.
vdiff report here shows the arrows no longer having a hover state, and unit tests updated to test no no events/interaction.