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

Combobox Component (Single Select) #1088

Closed
scurker opened this issue Jun 6, 2023 · 10 comments · Fixed by #1180
Closed

Combobox Component (Single Select) #1088

scurker opened this issue Jun 6, 2023 · 10 comments · Fixed by #1180
Assignees
Labels
enhancement New feature or request rfc An issue proposing a new significant change
Milestone

Comments

@scurker
Copy link
Member

scurker commented Jun 6, 2023

Follow general ARIA practices guidelines: https://www.w3.org/WAI/ARIA/apg/patterns/combobox/

This implementation addresses only single select comboboxes.

List items can be provided via a child component, ComboboxOption:

<Combobox label="Combobox">
  <ComboboxOption value="...">
    Item
    Longer Description
  </ComboboxOption>
</Combobox>

...or via a list with item definition:

<Combobox label="Combobox" options={[{ label: 'Foo', value: 'foo' }]} />

The label provided for Combobox should be of type ContentNode to allow for additional values beyond strings:

<Combobox label={`<Icon type="apple"/> Fruit`}>
  ...
</Combobox>

To allow for grouping of items, an additional ComboboxGroup component can be used to wrap combobox items:

<Combobox label="Combobox">
  <ComboboxGroup label="Group A">
   <ComboboxItem>1</ComboboxItem>
   <ComboboxItem>2</ComboboxItem>
   <ComboboxItem>3</ComboboxItem>
  </ComboboxGroup>
  <ComboboxGroup label="Group B">
   <ComboboxItem>4</ComboboxItem>
   <ComboboxItem>5</ComboboxItem>
   <ComboboxItem>6</ComboboxItem>
  </ComboboxGroup>
</Combobox>

For the list of items and ComboboxOption, providing the value property is optional and will reflect the value of the label associated with that item. Additionally, a selected prop can be provided to both to indicate that item is currently selected.

Under the hood:

  • The input component should behave similarly to <TextField /> (some different accommodations may need to be made to help support Combobox Component (Multiselect) #1103)
    • Include inner right padding to prevent text overflowing the down arrow
    • General input props should be supported such as disabled, required, etc.
  • Use the same styles as <OptionsMenuList> to display the list of items
  • Allow custom filtering function to display matching items on input via prop
    • Defaults to a text matching based on the the value or label of the item
  • Support both auto selection and manual selection during autocomplete (see #2 and #3 list items in the autocomplete section of the aria authoring practices)
  • When selected, an item should show it's selected state in the popup
    • This checkbox can reuse the existing check icon with a css ring to match the design
    • Changing the selected item(s) will call any onChange handlers

Other Notes:

Opening/closing the listbox has the following behaviors:

  • When Open
    • Has Selected Value (Autocomplete Manual)
      • The input value will be set to an empty string to allow users to see all options and filter options based on the combobox input
    • No Selected Value (Autocomplete Manual)
      • The input value will retain its initial value and will not clear, allowing the user to update the combobox input to change the filtered options
  • When Closed
    • Has Selected Value (Autocomplete Manual)
      • If the selected value did not change, the combobox input will revert back to the selected value
    • No Selected Value (Autocomplete Manual)
      • The combobox input will retain the currently typed characters

Examples:

light/dark versions of combobox

@scurker scurker added the enhancement New feature or request label Jun 6, 2023
@scurker scurker added this to the Q3 2023 milestone Jun 6, 2023
@scurker scurker changed the title Add Combobox Add Combobox Component Jun 6, 2023
@scurker scurker added the rfc An issue proposing a new significant change label Jun 6, 2023
@scurker scurker assigned scurker and unassigned bobbyomari Jul 5, 2023
@scurker scurker changed the title Add Combobox Component Combobox Component (Single Select) Jul 5, 2023
@thuey
Copy link
Collaborator

thuey commented Jul 6, 2023

To allow for grouping of items, an additional ComboboxGroup component can be used to wrap combobox items:

Is there a similar way to accomplish this using the item definition list approach?

As an aside, what is the value in supporting two ways to define the list?

@scurker
Copy link
Member Author

scurker commented Jul 7, 2023

As an aside, what is the value in supporting two ways to define the list?

It's similar to how we support options and HTMLOptions for selects and there's some value in keeping the props similar to make it easy to convert between the two:

options?: SelectOption[];
children?: React.ReactElement<HTMLOptionElement | HTMLOptGroupElement>[];

The general idea is if an author needs a simple list of label/value items they can use the list array. More advanced configurations could use the component to display more complex items.

As an example, React Spectrum (Adobe) Combobox does something very similar.

Is there a similar way to accomplish this using the item definition list approach?

Not unless we really think it's needed. I'm double checking with @dequelabs/cauldron-design on this, but my understanding is most cases will not need to be grouped so the list format will be the easiest to implement.

@schne324
Copy link
Member

<ComboboxGroup label="Group A">

This may already be planned (perhaps some prop types defined in the ticket would've cleared this up a bit) but I think the label prop for ComboboxGroup should support any react nodes so we can do stuffs like

<ComboboxGroup label={<Icon type="bananas" /> Fruits}>
   <ComboboxItem>Bananas</ComboboxItem>
   <ComboboxItem>Mangos</ComboboxItem>
   <ComboboxItem>Cucumbers (yes a cucumber is in fact a fruit)</ComboboxItem>
</ComboboxGroup>

Due to the complicated nature of comboboxes (and historical UA/AT bugs surrounding comboboxes), it may be useful to clearly document which screen readers we will support for this (in both the ticket and demo/docs page for it). I would suggest latest VO, NVDA, and JAWS versions as our target support here but maybe that is something to consider more "globally" (on the cauldron readme/landing page of docs list what AT we support)

@scurker
Copy link
Member Author

scurker commented Jul 13, 2023

<ComboboxGroup label="Group A">

This may already be planned (perhaps some prop types defined in the ticket would've cleared this up a bit) but I think the label prop for ComboboxGroup should support any react nodes so we can do stuffs like

Good callout, I can update the description.

<ComboboxGroup label={<Icon type="bananas" /> Fruits}>
   <ComboboxItem>Bananas</ComboboxItem>
   <ComboboxItem>Mangos</ComboboxItem>
   <ComboboxItem>Cucumbers (yes a cucumber is in fact a fruit)</ComboboxItem>
</ComboboxGroup>

Due to the complicated nature of comboboxes (and historical UA/AT bugs surrounding comboboxes), it may be useful to clearly document which screen readers we will support for this (in both the ticket and demo/docs page for it). I would suggest latest VO, NVDA, and JAWS versions as our target support here but maybe that is something to consider more "globally" (on the cauldron readme/landing page of docs list what AT we support)

There's already general guidelines here: https://cauldron.dequelabs.com/component-guidelines#accessibility

@Amandeque
Copy link

Estimates:
SIZE: Large

@Keerthi-Penukonda
Copy link

Keerthi-Penukonda commented Sep 8, 2023

Question

@scurker error message is being displayed even when selected an option from "Does this combobox have an error?" dropdown. Is this expected? or do we need to remove the error once the option is selected?

image

Testing

Verified UI and performed basic accessibility testing. Did not find any issues. Moving this to a11y column for accessibility testing.

@scurker
Copy link
Member Author

scurker commented Sep 8, 2023

@Keerthi-Penukonda Yes, that's expected. Combobox/text fields are dependent on the author clearing the error once there's a valid selection made.

@VaniChinnam
Copy link

Verified on the latest build. Raised 1 violation and 3 best practices.

Violation - 1505025
Best Practices - 1495130, 1505019, 1505022

@scurker
Copy link
Member Author

scurker commented Sep 21, 2023

Thanks @VaniChinnam, I've raised issues for the following:

For 1505022, it was an intentional design decision to allow for a user to easily filter down the list of options again if the user expanded the list again without being limited to options that were currently there. If the user types/remove characters the value does persist (or gets restored to its original value if no selections were made). I can ask @dequelabs/cauldron-design to see if we can get some usability testing here to see if this is something we want to tweak.

@Keerthi-Penukonda
Copy link

Closing this ticket as we have separate tickets to track accessibility issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rfc An issue proposing a new significant change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants