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

Add count prop to OuiTreeView #934

Open
2 tasks
KrooshalUX opened this issue Jul 28, 2023 · 6 comments · May be fixed by #1138
Open
2 tasks

Add count prop to OuiTreeView #934

KrooshalUX opened this issue Jul 28, 2023 · 6 comments · May be fixed by #1138
Assignees

Comments

@KrooshalUX
Copy link
Contributor

KrooshalUX commented Jul 28, 2023

  • Count prop should reflect the number of OuiTreeNode children, and should be optional to include
Screen Shot 2023-07-28 at 12 35 57 AM
  • Reference count styling from OuiFacet and OuiFilterGroup to build styling while retaining hover styling of OuiTreeView - spacing between text & count = $ouiSizeS
    Examples:
    - Default:
Screen Shot 2023-07-28 at 12 04 09 AM Screen Shot 2023-07-28 at 12 08 03 AM

- Disabled:
Screen Shot 2023-07-28 at 12 04 14 AM

Discuss:
What do we think about using the styling of selected that is present in OuiFacet and OuiFilterGroup for the isExpanded state of OuiTreeGroup ? Do we think that the expansion itself is visual communication or not? IMO, the expansion itself is sufficient, but for the sake of complete exploration here are examples of count changing color when a user is interacting with an element that contains a count:

Screen Shot 2023-07-28 at 12 08 55 AM Screen Shot 2023-07-28 at 12 09 03 AM
@pjfitzgibbons
Copy link
Contributor

Great design! Good re-use, showing the flexibility of facet as a base-component for other compositions.

A lot of my intuition on this is based on the absense of a left/down chevron, which the industry usually uses to show collapsed/shown accordion (and in fact is on Accordion). I am +1 on this absense, and I think the children-count indication is excellent and I feel is the thing that allows the exclusion of the chevron. Kind of traded-up the chevron for an equally appropriate indicator that gives more information.

Counter-intuitively, I would choose "selected" state for Collapsed tree segements. In my mind this would bring attention to things the user is not seeing. I feel like an expanded tree segment is highlight-enough in its own right.
I feel that color-changing the children-count is not desired. I think the color contrast would become a cognitive dischord, getting in the way of the understanding of the information itself - the tree and its content.

@pjfitzgibbons
Copy link
Contributor

@anirudha

@KrooshalUX
Copy link
Contributor Author

@pjfitzgibbons I may have caused some confusion by referencing facet styling here. OuiTreeView already exists and the expand / collapse icons can be set by the builder - they can be chevrons or other icons (like folders) : https://oui.opensearch.org/1.2/#/navigation/tree-view

Since count would be optional in OuiTreeNav, we would still want to use expand/collapse indicators. Builders may also choose to force expand the tree as well.

For clarity - this component doesn't contain facet - for the sake of speed/ not having to red-line a design or cause an inconsistency, I was pointing to the design of count on OuiFacet and OuiFilterGroup for the builder to reference.

Can you take a look at the OUI Docs for Tree view and see if the way icons for expand/collapse give you any concerns or questions ?

@pjfitzgibbons
Copy link
Contributor

OK. looking. You references "OuiTreeNav".. are you considering adding a new/fork component here? I don't understand the difference vs OuiTreeView.
And I didn't realize OuiTreeView was pre-existing... will investigate.

@KrooshalUX
Copy link
Contributor Author

@pjfitzgibbons No, not a fork or a new component, this is just an additional prop within the existing component. Right now there is no count in tree view today.

Here is what tree view looks like currently:

Screen Shot 2023-07-28 at 11 05 18 AM Screen Shot 2023-07-28 at 11 05 26 AM

@keskami
Copy link
Contributor

keskami commented Nov 2, 2023

I can take this issue. Basically a count prop that is displayed next to the labels of the tree view that mirrors OuiFacet's count prop?

keskami added a commit to keskami/oui that referenced this issue Nov 2, 2023
Signed-off-by: keskami <kkak808@gmail.com>
@keskami keskami linked a pull request Nov 2, 2023 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants