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

[Serverless] Project Settings Accordion doesn't expand/collapse from text label #168484

Closed
sixstringcode opened this issue Oct 10, 2023 · 14 comments · Fixed by elastic/eui#7337
Closed
Assignees
Labels
Project:Serverless Work as part of the Serverless project for its initial release Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)

Comments

@sixstringcode
Copy link

Screenshot 2023-10-10 at 6 58 02 AM

Observability and Elasticsearch projects have Project Settings collapsed by default. In both project types, the Project Settings label highlights on hover, but only expands by clicking the chevron to the right.

Screenshot 2023-10-10 at 6 58 24 AM

In expanded state, the same behavior exists, only the right chevron icon is actionable.

Screenshot 2023-10-10 at 6 58 31 AM

Expected: The entire highlighted hover area is clickable.

@sixstringcode sixstringcode added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Project:Serverless Work as part of the Serverless project for its initial release labels Oct 10, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@sebelga
Copy link
Contributor

sebelga commented Oct 10, 2023

@cee-chen Can you confirm if EUI allows the title of the accodion to control the toggle state?

@cee-chen
Copy link
Member

cee-chen commented Oct 10, 2023

It does not because accordion titles can also be links. IMO, it would be more confusing if users clicked on the title/text and for some of them it toggles an accordion, and others it navigates to a totally different page. Hence the UX preference here to make the arrow the single consistent toggle for collapsed content.

CC @MichaelMarcialis for his UX expertise here as well

@MichaelMarcialis
Copy link
Contributor

It does not because accordion titles can also be links. IMO, it would be more confusing if users clicked on the title/text and for some of them it toggles an accordion, and others it navigates to a totally different page. Hence the UX preference here to make the arrow the single consistent toggle for collapsed content.

CC @MichaelMarcialis for his UX expertise here as well

I share @cee-chen's thinking here. The original design intent for the navigation items was to restrict each to function as either an accordion or a link, but not both simultaneously. The reason for that restriction was to avoid the current behavior we have in the stateful Kibana navigation, where clicking a navigation item's text could toggle an accordion or take users to another page with no clear indicator as to which (ex. compare clicking the stateful nav's "Recently viewed" text versus the "Analytics" text).

CleanShot 2023-10-10 at 14 52 51

During development however, Cee engineered the serverless navigation to allow for links and accordions to exist in tandem for greater flexibility, but in a more consistent fashion than the current stateful navigation. This consistency was achieved by having the accordion toggles only applying to their related arrow buttons and links only applying to their related text. After we discussed it, we decided to keep it as Cee had engineered due its overall greater flexibility.

Ultimately, if the solution teams want the ability for a single navigation item to allow for both links and accordion toggles simultaneously, then I think we should proceed as is and monitor for user feedback. However, if solution teams don't want the ability for individual navigation items to support these dual capabilities, then we can discuss simplifying it to the original design intent (i.e. clicking nav item text supports either link or accordion, but not both). CCing @ryankeairns in case he has any insights on this.

@sebelga
Copy link
Contributor

sebelga commented Oct 23, 2023

I am slightly confused here 😄

In my opinion, an accordion header title should only do one thing when clicking on it: toggle the visibility of its content. I would never expect an accordion title to open a link.

I opened a PR to allow different navigation groups in the side nav: #169251
I can imagine the "block group" bold title to be a link (rendered as such with blue color and maybe underlined)
I can not imagine the "accordion group" title to do anything else than toggling the accordion.

As EUI controls the rendering of the accordions, I think the default behaviour should be that the title controls the collapsible state and maybe add a prop for consumers to opt out if they really don't want that behaviour.

@cee-chen
Copy link
Member

In my opinion, an accordion header title should only do one thing when clicking on it: toggle the visibility of its content. I would never expect an accordion title to open a link.

I'm fine if we want to make that breaking UX decision, but FWIW this was implemented to preserve the existing behavior of Kibana's nav:

nav

A secondary consideration (not a blocker, but just something to note) is that having the accordion trigger be a link negates the need for a "Home" link for each section, which we'd otherwise have to render.

@sebelga
Copy link
Contributor

sebelga commented Oct 25, 2023

I'm fine if we want to make that breaking UX decision, but FWIW this was implemented to preserve the existing behavior of Kibana's nav

Interesting 🤔 Thanks for pointing that out. I will leave the decision to design, the only thing would be to be consistent (not mixing behaviours).

If we look at the nav items that open a panel. They all have the icon to open the panel + a link that points to a landing page (containing the links of the panels in this case).
We could have that same pattern for accordions, the title opens either a "home" page or a landing page and the chevron expands/collapses the accordion.

Captura de pantalla 2023-10-24 a les 12 45 31

@cee-chen
Copy link
Member

the only thing would be to be consistent (not mixing behaviours).

That was my reasoning for making only the arrow trigger the accordion open/close, and not the title/text. As a user, I would be fairly annoyed if sometimes clicking text triggered an accordion and sometimes it took me to a page instead - I'd have no idea what to expect.

Whereas with the current UX, it's very clear if the title/text is a link or not (underline on hover) and the action to open the accordion is always the same (clicking the arrow).

To be honest, I really don't love the "opens another panel" pattern that security currently uses and I don't consider it to be a pattern that we want to continue to adopt across Kibana. CC @ryankeairns for more thoughts, maybe I'm off on that

@sixstringcode
Copy link
Author

Following this thread it sounds like we are considering this by design. I strongly disagree. The whole section highlights when you hover. It's not clear that it only expands using the chevron on the right. It doesn't make sense to have 90% of the clickable space dead for something so heavily used.

@cee-chen
Copy link
Member

cee-chen commented Oct 26, 2023

I don't disagree, but I also feel very strongly that the title sometimes being a page link and sometimes being an action/expansion trigger is not good or consistent UX.

If we think both scenarios aren't great UX-wise, maybe we should consider going with the approach Marcialis first proposed in his designs: group titles can never be links, and are always accordion triggers, and if certain section headings have pages attached to them, they should go into the nested children as a "Home" link or similar.

As a warning this will require a certain amount of IA (information architecture) shuffling for Kibana's existing navs, as noted above, they do not follow this rule currently.

@ryankeairns
Copy link
Contributor

The secondary/flyout nav is here to stay, and it provides the two-action capability. Given that, I feel the accordions should just be accordions with a single action (i.e. both the text and icon toggle the accordion).

We are capturing these (and other topics) in a set of UX guidelines for this navigation design and how to implement it. For this specific case, those likely look like:

  1. If you desire two actions (i.e. the text link to open a page), then you should use the secondary/flyout pattern
  2. If you want to use an accordion, then the text link and icon both toggle the collapsed section

In other words, accordions do not have two actions. They only provide accordion behavior.

As for the version in Stack (not Serverless) and those related components - I anticipate we'll be moving away from that design and providing a solution-focused experience as in Serverless. In other words, that need to align to current behavior will likely go away by Kibana 9.

@cee-chen
Copy link
Member

The secondary/flyout nav is here to stay, and it provides the two-action capability

Just to confirm Ryan, should I bake that into the EUI component by default in that case?

In other words, accordions do not have two actions. They only provide accordion behavior.

👍 Should I go ahead and make this change in EUI as well to remove links from the accordion titles? Or do we need to wait at all to make sure Kibana is ready for that nav infrastructure change?

@ryankeairns
Copy link
Contributor

ryankeairns commented Oct 27, 2023

The secondary/flyout nav is here to stay, and it provides the two-action capability

Just to confirm Ryan, should I bake that into the EUI component by default in that case?

Very likely, yes. That said, we have this built on main and are not blocked (i.e. its not urgent to do so). The dust is settling, and we should chat with @sebelga and see how that all panned out, what thoughts he has, etc. In other words, there is some review that would be a wise pre-requisite, imo.

In other words, accordions do not have two actions. They only provide accordion behavior.

👍 Should I go ahead and make this change in EUI as well to remove links from the accordion titles? Or do we need to wait at all to make sure Kibana is ready for that nav infrastructure change?

🤔 I am curious your thoughts on if/whether there is an option to enforce this only within the new design? If I'm understanding your question, removing the links from accordion titles - writ large - would impact how the current Elastic Stack nav group headings work (in EuiCollapsibleNavGroup)... right?

@cee-chen
Copy link
Member

🤔 I am curious your thoughts on if/whether there is an option to enforce this only within the new design?

Yep, 100% possible because we created a new component instead of modifying the old one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Project:Serverless Work as part of the Serverless project for its initial release Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants