Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/tree view improvements #2840

Merged
merged 29 commits into from
Jan 10, 2023
Merged

Feat/tree view improvements #2840

merged 29 commits into from
Jan 10, 2023

Conversation

rustem-nasyrov
Copy link
Collaborator

Close #2639
Close #2638
Close #2545
Close #2661

Description

  • Keyboard navigation
  • Refactoring and improvements
  • VueBook: added demo for testing keyboard navigation

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

@rustem-nasyrov rustem-nasyrov added docs packages/docs component Is a new component or part of existing one labels Jan 4, 2023
@rustem-nasyrov rustem-nasyrov self-assigned this Jan 4, 2023
@rustem-nasyrov rustem-nasyrov marked this pull request as ready for review January 4, 2023 15:49
Copy link
Collaborator

@m0ksem m0ksem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for me. Have you found a guide for keyboard navigation somewhere? I would like to select node with space, because for now I need to select checkbox with tab and then go back.

If you used some guide, for example, w3c, how about mentioning it in docs?

@rustem-nasyrov
Copy link
Collaborator Author

Looks good for me. Have you found a guide for keyboard navigation somewhere? I would like to select node with space, because for now I need to select checkbox with tab and then go back.

What you think about this scenario:

  • Pressing on space key will emit selected event while tree view isn't in selectable mode.
  • Pressing on space key will toggle checkbox (selectable mode).

If you used some guide, for example, w3c, how about mentioning it in docs?

I've used this guide from MDN.

@m0ksem
Copy link
Collaborator

m0ksem commented Jan 5, 2023

Looks good for me. Have you found a guide for keyboard navigation somewhere? I would like to select node with space, because for now I need to select checkbox with tab and then go back.

What you think about this scenario:

* Pressing on `space` key will emit `selected` event while tree view isn't in `selectable` mode.

* Pressing on `space` key will toggle checkbox (`selectable` mode).

If you used some guide, for example, w3c, how about mentioning it in docs?

I've used this guide from MDN.

So let's use space as described here: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tree_role#multi-select_keyboard_interactions

Copy link
Contributor

@aluarius aluarius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, i've done with code. Next i'll leave a few suggestions related to fucntionality.

@aluarius
Copy link
Contributor

aluarius commented Jan 9, 2023

I've tried space and enter to expand it and just eventually realized, that here used right-left. Imo should be updated. But can't find suitable specification - only for navigation treeview, but this component is common, not only for navigation.

image

@aluarius
Copy link
Contributor

aluarius commented Jan 9, 2023

I give up :) All specifications say that enter-space should select node. But for me it looks unobvious.

@rustem-nasyrov rustem-nasyrov merged commit cd32ee2 into epicmaxco:develop Jan 10, 2023
@rustem-nasyrov rustem-nasyrov deleted the feat/tree-view-improvements branch January 10, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component Is a new component or part of existing one docs packages/docs
Projects
None yet
4 participants