Add Combobox component to Toucan Core & Toucan Form

[ynotdraw]

🚀 Description

This PR builds on top of the work @clintcs had done on the select component. It was decided to make this a combobox instead. I apologize how huge this PR is, there was already a lot going on and I wanted to make sure everything worked end-to-end.

API Considerations

• Should we rename @onChange to @onSelect
  • Could be confusing, as it's a textbox/input and we only trigger the event when an option is selected

@optionKey
At the start of this component, it was thought that @optionKey was overkill and a component API smell; however, it turns out it has some advantages. You can see where I changed from filterBy to optionKey+onFilter in this commit.

Here is how @optionKey makes our lives easier:

• Search/filtering baked in is easier
• We need to keep track of the previous selected value anyway, so that we can reset it under certain conditions
• We need a way to display what the selected option is inside of the input
  By using @optionKey, we can set the value of the input to selected[optionKey] and use optionKey in the default search scenario.

Checklist of functionality / features post-handoff from Clinton

• Add aria-expanded (a11y)
• Add role="combobox" (a11y)
• Add aria-activedescendant (a11y)
  • This allows the screenreader to announce the option as you up/down arrow through the options
• Add default search
• Make selected state internal to select
• Add @optionKey
• Handle case Clinton described when clicking away after typing should reset to the previously selected value
• Fix up Select Field to not have errors and to work as we expect
• Write docs
• Handle case where there's input text and we re-click the combobox, it shouldn't filter, just show all items
• Make it so that when you click the select, it highlights the input value like in mui's combobox
• Add a no results message
• Space probably should not select the option in case there are spaces in the text?
• Make styles match designs. In particular the hover+active colors
• Make it all work in toucan-form (aka expose the select-field + ensure it works as we expect)
• Rename from select to combobox
• Write Control tests
• Write Field tests
• Write Form tests
• Check in pnpm-lock changes

---

🔬 How to Test

I've written a ton of tests on the usability and logic for how this should all work from a UI/UX perspective based on our chats with design. Please go check out those tests and play with the Combobox preview URLs!

To test via preview URLs

• Navigate to https://bd96031b.ember-toucan-core.pages.dev/docs/components/combobox
  • Mouse
    • Click the Colors select control
    • Verify the list of options opens and is displayed
    • Type b
    • Verify only blue is shown
    • Click "Blue"
    • Verify the popover closes and "Blue" is selected
  • Keyboard
    • Click the Colors select control
    • Verify the list of options opens and is displayed
    • Type b
    • Verify only blue is shown
    • Press the ENTER key
    • Verify the popover closes and "Blue" is selected

To test things locally

(optional)

Testing locally is optional, as there are a ton of tests written for the functionality and the preview URL should suffice for testing usability; however, if you want to:

pnpm i
# I recommend building first
# as I hit some issues adding a new component where
# toucan-form would get confused
pnpm build
# Start the servers
pnpm start

• Navigate to http://localhost:4201/docs/components/combobox and play with the combobox
• Navigate to http://localhost:4201/docs/components/combobox-field and play with teh combobox field
• Navigate to http://localhost:4201/docs/toucan-form/changeset-validation
  • Click submit
  • Verify the combobox is in the error state
  • Select an option
  • Verify the error disappears
  • Fill out the rest of the form and verify an alert is shown

---

📸 Images/Videos of Functionality

Screen.Recording.2023-07-07.at.10.09.29.AM.mov

Screen.Recording.2023-07-07.at.10.13.14.AM.mov

Screen.Recording.2023-07-07.at.10.14.16.AM.mov

[changeset-bot]

🦋 Changeset detected

Latest commit: 212cd77

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@crowdstrike/ember-toucan-core	Patch
@crowdstrike/ember-toucan-form	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Preview URLs

Env: preview
Docs: https://9d8a37a9.ember-toucan-core.pages.dev

[simonihmig]

Approved, with some suggestions and remarks. I'm totally fine with either dismissing them if you disagree, or tackling them in future iterations, as I don't want to block this big PR any further...

[simonihmig]

Commonly used to specify a z-index.

Do users actually have to do this? This is no headless component, instead we do apply everything that's needed to make it look right. So shouldn't this "just work"(TM)?

[ynotdraw]

More info over on #200 (comment), but hopefully this is temporary and we can remove it soon™. We just need to figure out where/if some of the CSS variables we have should be defined!

[simonihmig]

Do we have to add it here? The addon already has it as a dependency. so that should be enough?

[simonihmig]

Why do we need explicit mouseover support here? We have ...attributes, so any event listener can be added? The @onClick support seems to be needed for event.preventDefault(); to be baked in. But this doesn't have it, so users could just apply {{on "mouseover"}}, right?

[ynotdraw]

Good question! Yeah, I think we probably could.

This gets used in combobox.hbs where we yield back Option. We do the following:

onMouseover=(fn this.onOptionMouseover index)

Is there a way to make this a modifier rather than a component argument when we are yielding back?

[simonihmig]

Nit: If it's not possible to be in that state, then we can use assert(), which makes TS happy and does not add runtime code (in prod).

[ynotdraw]

Perfect! Pushed as part of commit 4f2d139

[simonihmig]

Nit: would have been enough to just not comment out the embroider scenarios in ci.yml as you did, and leave this as-is...

[ynotdraw]

Fixed with 0c8f958

[simonihmig]

testarea? 🙃

[ynotdraw]

Thank you! Turns out a lot of copy pasta (ac3991c)

[simonihmig]

I saw we are calling this with ´await`, so should be this?

Suggested change

	onFilter?: (input: string) => OPTION[];
	onFilter?: (input: string) => OPTION[] | Promise<OPTION[]>;

[ynotdraw]

Ah yeah, I backed out of it being a promise for now due as the current implementation is not promise based. Let me revert the await! We can add that support back if we find out folks need it! d036a43

[simonihmig]

Should we also pass the raw options here, so the function can be simple and not need to have a of closure to have access to the options? Like

Suggested change

	filteredOptions = await onFilter(value);
	filteredOptions = await onFilter(optionsArgument, value);

Another option (pun intended) would be to make this function a higher order function that returns the filter function, so you could do:

Suggested change

	filteredOptions = await onFilter(value);
	const filterFn = onFilter(value);
	filteredOptions = optionsArgument.filter(filterFn);

We lose the ability for async though. But it would have the benefit that you can only return options that are actually members of @options. Which would be weird if that wasn't the case, and likely cause problems (e.g. when doing === checks). In the suggestion above such a mistake would be less likely, and more likely in the original implementation here.

I don't have strong opinions on this though...

[ynotdraw]

Yeah for sure, I could see this being helpful. I'm going to go ahead and merge as-is if that's okay due to our protection with changesets and am going to chat up @clintcs on Monday about this!

[simonihmig]

This can be marked as optional, right? Maybe also point out what the default filtering logic does, so it's clear you don't need this if the defaults work as well.

[ynotdraw]

Just wrote some more as part of 2b74de0, thank you!

[simonihmig]

My earlier comment here was a bit unemotional or sober, and concentrated only on possible improvements without stating the most important thing: this is some absolutely beautiful work here! Congrats @ynotdraw and @clintcs! 🎉 😅