AlaskaAirlines / auro-accordion

Custom element that allows users to toggle the display for sections of content
https://auro.alaskaair.com/components/auro/accordion
Apache License 2.0
1 stars 1 forks source link

auro-accordion: blueprint #44

Closed braven112 closed 1 year ago

braven112 commented 2 years ago

Blueprint & Figma Component

We need a blueprint for auro-accordion that matches the features we have currently on our doc site.

Outline tasks

Optional

Note: We don't need the shade option

Specific steps required to publish this component to Figma library and Auro doc site.

  1. Design & Planning

    • [ ] Blueprint is created
  2. Development

    • [ ] Sync with devs and get feedback and to make sure features in the blueprint match what will be developed in the first milestone.
    • [ ] Archive any features that will need to be part of a future version.
  3. Documentation

    • [ ] Component properly described
    • [ ] Component use case(s) outlined
    • [ ] Do Not section outlines implementation scenarios to avoid
    • Example Section
      • [ ] Basic example of simplest use case
      • [ ] Work with dev to add other common use cases
      • [ ] Add figma instructions
      • [ ] Create release notes for new component
      • [ ] Add Figma tab and figma examples
  4. Publishing

    • [ ] Publish component to figma library
    • [ ] Create PR for release on doc site
leeejune commented 2 years ago

https://www.figma.com/file/jFPo0zwVENQVeOauFyHVuF/Accordion?node-id=846%3A12442

Blueprint done. Waiting for feedback on behavior from Nick King.

blackfalcon commented 2 years ago

This focused state is incorrect.

Screen Shot 2022-04-29 at 1 01 10 PM

Looking at the docsite, when the trigger has tab focus, this is the UI

Screen Shot 2022-04-29 at 1 02 05 PM
leeejune commented 2 years ago

The blueprint is for the "new" design. The "current" version is here: https://www.figma.com/file/VpUz89Ov6ImBpY5YvzYbZW/Auro-toolkit?node-id=2441%3A15317

leeejune commented 2 years ago

After a conversation with @blackfalcon, I will be adding other default options for more flexibility.

leeejune commented 2 years ago

@blackfalcon Added descriptions for mods and variations: https://www.figma.com/file/jFPo0zwVENQVeOauFyHVuF/Accordion?node-id=1008%3A16292

blackfalcon commented 2 years ago

@leeejune this looks great! I see animation and a11y are not checked from the options above.

We should have some kind of reference as to the animation of the accordions. What moves, when and how fast, would cover it.

For a11y, please note what the expected UI is to be when tabbed to an accordion. You are designating a focused state, but is that a focus-visible state or an actual focus event when a user is also clicking on the trigger?

jason-capsule42 commented 2 years ago

@leeejune Since the trigger is essentially a button I think we need an active state.

jason-capsule42 commented 2 years ago

re: the icon type modifier - is the idea here that it's a slot that they can put any icon into or that we have a defined list of options for them to choose from?

jason-capsule42 commented 2 years ago

Should the anatomy section use the default layout which would include the divider?

Also the anatomy section refers to the "chevron" where the modifiers section refer to the "icon". Because these were different I thought at first that icon meant adding a second icon into the header, similar to the badge. It looks like it's actually both. But it took me a moment to figure it all out. Maybe we call this particular icon the "state icon" or something and use that name in all locations we point it out?

jason-capsule42 commented 2 years ago

padding modification - are we going for preset options (e.g. padding="xs|sm|md|lg|xl" or just exposing this via CSS parts and letting them use whatever they want?

jason-capsule42 commented 2 years ago

It seems like the extra badge and second icon modification are just additional content in the header slot? Or is this extra icon and badge supposed to be in our template but only shown optionally?

In the side navigation example, all examples have the extra icon but only one of them calls it out as a modification. What should happen if someone sets up this example where only one of them does have the extra icon does the text for all of these accordions stay aligned with space for the icon or no?

jason-capsule42 commented 2 years ago

Using the word "single" to identify an accordion that does not stretch to fill the available width throws me off a little. The name makes me think it's meant to do something more than that. Am I missing something?

jason-capsule42 commented 2 years ago

It appears that the only difference between small|medium|large sizes is the padding in the header. It would be good to have consistent API patterns for similar affects across our components. In dropdown we did this using an attribute called "inset".

jason-capsule42 commented 2 years ago

Looking a the vertical indicator examples -

I'm thinking of a case where multiple accordions are on a single page of text content (so they are all left aligned on the page the same) and some of have the indicator and some don't. In this case, the actual text of the accordion would not be left aligned to the same point.

Should all accordions have the space allocated for the indicator (but transparent) and only make it visible (blue) when the attribute is set?

jason-capsule42 commented 2 years ago

Thinking about accessibility - I think we should have a defined text reader string that announces the expanded|collapsed state of the accordion when focused on the header. How are we making it clear that the content being read by the screen reader is content inside the accordion and when the exit the accordion and are readying content outside of it? Thoughts? @geoffrich

jason-capsule42 commented 2 years ago

Thought regarding customizing the state icon with a different icon - I know there was a discussion about adding an animation to the auro-dropdown chevron once we landed on a transition pattern. I would imagine we would do the same here? Would that animation only apply when using our default icon then? Seems the answer there depends on if we are predefining a set of state icon options or using a slot to allow them to put whatever they want in there.

blackfalcon commented 2 years ago

Given this long list of comments, @leeejune please schedule a meeting with @jason-capsule42 and I to review the Figma proposal.

blackfalcon commented 2 years ago

@jason-capsule42 or @leeejune can either one of you summarize the outcomes of the meeting on Monday the 20th?

leeejune commented 2 years ago

These are comments made after having a discussion with @jason-capsule42 and @blackfalcon.

@leeejune Since the trigger is essentially a button I think we need an active state.

Accordion would not have an active state.

re: the icon type modifier - is the idea here that it's a slot that they can put any icon into or that we have a defined list of options for them to choose from?

The default is "chevron" but it is up to the designer to choose an icon.

Should the anatomy section use the default layout which would include the divider?

The anatomy section simply outlines all of the parts of a component. The default for designers will be type "default" and for engineers it might look different (no divider).

But it took me a moment to figure it all out. Maybe we call this particular icon the "state icon" or something and use that name in all locations we point it out?

Edits made to address this issue.

padding modification - are we going for preset options (e.g. padding="xs|sm|md|lg|xl" or just exposing this via CSS parts and letting them use whatever they want?

The padding can have preset options which would correlate to our design tokens.

It seems like the extra badge and second icon modification are just additional content in the header slot? Or is this extra icon and badge supposed to be in our template but only shown optionally?

It is shown optionally; it is part of the slot/custom content.

In the side navigation example, all examples have the extra icon but only one of them calls it out as a modification. What should happen if someone sets up this example where only one of them does have the extra icon does the text for all of these accordions stay aligned with space for the icon or no?

The extra icon is listed part of the "custom content" in the trigger slot.

Using the word "single" to identify an accordion that does not stretch to fill the available width throws me off a little. The name makes me think it's meant to do something more than that. Am I missing something?

The naming was meant for designers. It can be named something different if it makes more sense to do so. What the intention was is that the "single" accordion would have no flex property as a separate variant. The "default" and "vertical indicator" types would have the full width flex property.

It appears that the only difference between small|medium|large sizes is the padding in the header. It would be good to have consistent API patterns for similar affects across our components. In dropdown we did this using an attribute called "inset".

Check again to see the differences in font size as well as the padding.

Should all accordions have the space allocated for the indicator (but transparent) and only make it visible (blue) when the attribute is set?

This is a developer choice that I have no designer opinion on!

Thought regarding customizing the state icon with a different icon - I know there was a discussion about adding an animation to the auro-dropdown chevron once we landed on a transition pattern. I would imagine we would do the same here? Would that animation only apply when using our default icon then? Seems the answer there depends on if we are predefining a set of state icon options or using a slot to allow them to put whatever they want in there.

Animation would not be part of the accordion due to the customizable characteristic of the icon.

blackfalcon commented 1 year ago

@leeejune I understand that the design for this element is under review? Is this issue still valid or should we create a new issue to track that work?

leeejune commented 1 year ago

I'm not sure if @jason-capsule42 was done with review. We are considering redesigning the experience, so I think we can track the work in a new issue. Will be closing this one.