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

[Feature Request] Make Selectable Treeview look and behave the same as Select #7475

Closed
aantipov opened this issue Jun 12, 2019 · 6 comments
Closed
Labels
C: VTreeview VTreeview S: has PR The issue has a pending Pull Request T: enhancement Functionality that enhances existing features
Milestone

Comments

@aantipov
Copy link

Problem to solve

It's possible to use Treeview component as a select component with checkboxes to select some values from a tree.
The problem is that Treeview looks and behave differently compared to Select component.
If your form contains both normal Select component and selectable Treeview, than the user has a mixed user experience:

  • in Treeview you can't click on a label to select a value
  • the labels' font and size is different
  • the spacing is different
  • the on-hover background color is different
  • no on-click animation when selecting items in Treeview

Compare https://codepen.io/alexeime/pen/EBaLeg and https://codepen.io/alexeime/pen/rEaoaE?&editable=true&editors=101

Select Component

select

Selectable Treeview Component

treeview

Proposed solution

Make selectable Treeview look and behave the same as Select component

@ghost ghost added the S: triage label Jun 12, 2019
@jacekkarczmarczyk
Copy link
Member

jacekkarczmarczyk commented Jun 12, 2019

It's little bit different in 2.0, but

in Treeview you can't click on a label to select a value

Because you open the subtree when click the label. But I'd remove the pointer cursor on tree leaves as click on the leaf label does nothing (at least when openOnClick === true)

the labels' font and size is different

Agreed, in treeview looks too big imho

the spacing is different

48px height doesn't look bad and might be better for mobiles in terms of accessibility, we could also add dense prop that would make it 40px

the on-hover background color is different

Agreed, should be same, actually imho it's too dark for v-list (opacity: 0.2 while in most places we're using opacity: 0.12)

no on-click animation when selecting items in Treeview

Agreed, could be both ripple for the whole item as well as the checkbox (the exact behaviour may depend on some props like openOnClick, but generally i think it's a good idea)

With few changes in css both could look like this (it's not a fully working demo):
https://codepen.io/anon/pen/QXbKRR - changed only hover background color
https://codepen.io/anon/pen/RzPGvQ

@jacekkarczmarczyk jacekkarczmarczyk added T: enhancement Functionality that enhances existing features C: VTreeview VTreeview and removed S: triage labels Jun 12, 2019
@jacekkarczmarczyk jacekkarczmarczyk added the S: has PR The issue has a pending Pull Request label Jun 14, 2019
@jacekkarczmarczyk
Copy link
Member

I've added PR with some proposed changes, the difference between treeview and list hover background color should be covered in #7519
The ripple thing may be discussed in a separate issue

@aantipov
Copy link
Author

Your speed is amazing, man. Didn't expect that my proposal will be heard and implemented so quick.
My respect 👍
Sorry, didn't have time to reply.

Because you open the subtree when click the label. But I'd remove the pointer cursor on tree leaves as click on the leaf label does nothing (at least when openOnClick === true)

I would rather do select/deselect when clicking on a leaf label (the user does know if it is a leaf or not a leaf. If a user clicks on a leaf, he does it on purpose, he expects it to be selected/deselected), or at least have a configuration option to be able to do it.

48px height doesn't look bad and might be better for mobiles in terms of accessibility, we could also add dense prop that would make it 40px

Agree, but the 34px height in Treeview is too low and I think it should be consistent with Select - either 48px or densed 40px

With few changes in css both could look like this (it's not a fully working demo)

In my application I also implemented required css changes and also select/deselect when clicking on a leaf node, so that Select and Treeview have the same look and feel.
But of course it's desirable to have it out of the box.

@jacekkarczmarczyk
Copy link
Member

If a user clicks on a leaf, he does it on purpose, he expects it to be selected/deselected), or at least have a configuration option to be able to do it.

This is something that I'd consider as a useful feature, but let's try to minimize a number of changes per issue/PR. So please create a separate issue for each problem that is not solved by one of these 2 mentioned PRs (#7518 fixes spacing/sizing/font-size, also adds dense prop and text overflowing, #7519 unifies the background color of hovered/selected item)

@aantipov
Copy link
Author

Sure. Here is the issue for clickable labels #7523
Thanks a lot for your work!

jacekkarczmarczyk added a commit that referenced this issue Jun 18, 2019
BREAKING: Must now use the dense prop to get previous styling

* fix(VTreeview): line-height, text-overflow, spacing

* fix(VTreeview): layout with append/prepend icons

* docs(VTreeview): dense mode example

* test(VTreeview): added dense snapshot

* fix(VTreeview): rtl issue

fixes #7475
@jacekkarczmarczyk
Copy link
Member

CSS issues fixed by #7518

@johnleider johnleider added this to the v2.0.0 milestone Jun 18, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: VTreeview VTreeview S: has PR The issue has a pending Pull Request T: enhancement Functionality that enhances existing features
Projects
None yet
Development

No branches or pull requests

3 participants