- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6.8k
fix(cdk-experimental/ui-patterns): enter/space/click in single selection mode should not deselect tree item #31843
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
Conversation
|  | ||
| if (!this.multi() && !this.followFocus()) { | ||
| return manager.on(e => this.goto(e, {toggle: true})); | ||
| return manager.on(e => this.goto(e, {selectOne: true})); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this just be combined together? I am not sure if I understand why we change this to toggle vs selectOne? Is it because it isn't mulit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, when it's not multi-selectable clicking on the same tree item shouldn't deselect it https://www.w3.org/WAI/ARIA/apg/patterns/treeview/examples/treeview-1a/ so it should perform selectOne instead of toggle.
The current behavior https://ng-comp-devapp.web.app/cdk-experimental-tree is incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry - I am still a noob at github - I meant lines 279 is also doing the same thing. So essentially it just would need to check if it is not multi.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh oops yes you are absolutely correct. It should be combined to
if (!this.multi()) {
  return manager.on(e => this.goto(e, {selectOne: true}));
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Thanks for catching that redundant code!
…ion mode should not deselect tree item
| This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. | 
No description provided.