**Feature:** Tree a11y #1168
**Feature:** Tree a11y #1168
Conversation
|
|
||
| Try it yourself! | ||
|
|
||
| - "Region" has a double click/<kbd>Enter</kbd> handler |
There was a problem hiding this comment.
Region reacts to single "Enter" keystroke with the double-click handler for me. Hence, I also can not close/open that part of the tree with my keyboard (single clicking with mouse still works as expected though)
There was a problem hiding this comment.
Space should handle expand and collapse, while enter triggers the “action” on double click. This is outlined in the README.md. 😉
I made sure to include it. https://github.com/contiamo/operational-ui/pull/1168/files#diff-da0c8b02385f203827104fc313ccb636R10
There was a problem hiding this comment.
Perhaps I could reword things to make them a little clearer. Or do you have an idea to make the behavior more like what you expect?
There was a problem hiding this comment.
I have another question. The item "Legal Entity" right below the "Region" reacts to "Enter" key with opening/closing. It feels like a pretty inconsistent behaviour to me. I would expect to use Space key only everywhere for opening/closing the tree items, especially since there is no visual sing or marker, telling me, that "Region" has a handler for a double-click/enter attached. What's your take on this @TejasQ?
There was a problem hiding this comment.
Apart from the click/space/enter inconsistency, the rest looks good to me, I would leave it as it is 👍
There was a problem hiding this comment.
I think we could leave that up to the users. I'm also not that convinced about the behavior. It is currently:
- if it's foldable, space toggles, enter double-clicks
- if no double-click specified, enter falls back to click
- if no click specified, #yolo
- if no double-click specified, enter falls back to click
I totally understand what you're talking about but I figure we'll have an issue about it from a more concrete use case in the future if its truly confusing. I think it's a little hard to say at this stage.
TejasQ
dismissed
mpotomin’s stale review
Changes requested on expected behavior.
|
Sometimes I get following behaviour on double click:
Maybe a race condition or something |
src/Tree/ChildTree.tsx
Outdated
| } | ||
| clickCount.current++ | ||
| setTimeout( |
There was a problem hiding this comment.
Do we want to cancel previous timeout before launching new one?
|
@stereobooster awesome. I think it's maybe that the timeout is not long enough. 😢 Unfortunately if I make it longer, the UX seems choppy. I don't really know how to solve it. I'll make an issue. |
| () => { | ||
| if (clickCount.current === 1) { | ||
| if (hasChildren) { | ||
| setIsOpen(!isOpen) |
There was a problem hiding this comment.
This theoretically can be triggered after component unmount, which will trigger the error
| return | ||
| } | ||
| }, | ||
| [onNodeContextMenu, onNodeDoubleClick, onNodeClick], |
There was a problem hiding this comment.
| [onNodeContextMenu, onNodeDoubleClick, onNodeClick], | |
| [onNodeContextMenu, onNodeDoubleClick, onNodeClick, onRemove], |
we need linter for this
|
Why unit-tests are not played on this PR 🤔 Is it new? |
|
The double click action fold the tree section for me (and expanded when I close the alert) |
|
Side question: how do you trigger context menu with keyboard? |
|
@stereobooster please read the PR (see highlighted line). |
|
So there's no clear direction on how to handle use cases with both double click and right click behavior. I will close this and focus on other stuff until it makes sense to spend time here again. |
|
@TejasQ We'll make figuring this out a priority. I'm sure some of this PR will live on 👍 . |
Summary
This PR:
We:
onDoubleClickRelated issue
To be tested
Me
localhost:6060Tester 1
Tester 2