Bug: TokenSelectDropdown is not accessible

Describe the bug and the steps to reproduce it

The TokenSelectDropdown component does not follow the Listbox WAI-ARIA spec https://www.w3.org/WAI/ARIA/apg/patterns/listbox/

For example, when the component is open, pressing up/down arrow keys doesn't select values while it should.

Also the component name is not right because it's not a dropdown. Options are just values and don't trigger actions (filter, sort etc...). The component should be renamed to TokenSelect.

I would suggest to use the Select component from Radix as it already follows the spec. Imo other components should also use Radix primitives as they take care of all the accessibility stuff and allow us to focus on the business logic. Other big libraries like Shadcn are also building on top of it.

I can work on this, just need to make sure we are aligned regarding introducing Radix.

Also I see the Swap component is following what we discussed about in a previous PR. In this case I'll go with an API similar to the Select component from Shadcn.

What's the expected behavior?

Be accessible and have the correct name

What version of the libraries are you using?

0.23.4

Great callout here.

Naming

I agree with you on the naming. TokenSelect* is a grouping of three components. If TokenSelectDropdown name is changed, current TokenSelectButton (button) and TokenSelectModal (modal) names need to be updated to match.

Accessibility

Other than the name rotation: TokenSelectDropdown should use select and option instead of using div and button. Specifically, TokenSelect needs to be select and TokenSelectDropdown needs to use option

I wanted to improve these but ran out of time.

Radix

There were extended conversation about integrating radix. Long story short, the decision was not integrating with radix.

However, the intent is to allow users to use UI library of their choice.

Overriding UI library

Swapping out underlying components for *uncontrolled components like Token components is straight forward. At most, Token components take three props: token, setToken, options. With Swap component, story becomes much more complex.

Let's dial up the complexity and talk about:

UI component placement
Share control logic

Scenario 1 - Original Swap + Token override components

Overriding Token component is straight forward. But what if the user wants to use Token override components with SwapAmountInput?

Few potential approaches are:

Composable SwapAmountInput component API like the top level Wallet and Swap component.
Use hooks + context to provide enough information for the user to build SwapAmountInput from the ground up.

Scenario 2 - Override everything

Take a look at this or latest main branch for Swap components - https://github.com/coinbase/onchainkit/pull/726

This PR refactors all business logic to reside at the root Swap component. This allows Swap subcomponents to receive everything via hooks + context.

Either all the hooks are exposed or OCK allows component whatever override method used for uncontrolled components.

<Swap>
  <SwapAmountInput as={MySwapAmountInput} />
</Swap>

or something like

function SwapAmountInput() {
  const { ... } = useSwapData('textinput:from')
}

I will stop right here but there is quite a few ways to enable to override the entire UI layer. I recommend to explore various methods to achieve the goal of "bring your own UI library".

To make sure I'm following you correctly. Your goal is to allow users to compose their own components using:

1) Subcomponents of the given component i.e. build TokenSelect using TokenSelectRow, build Swap using SwapAmountInput etc... 2) Written from scratch components by users i.e. build Swap where SwapAmountInput is actually written by the user. 3) Subcomponents from other components i.e. build TokenSelect using SwapAmountInput

Assuming that's the direction, points 2 and 3 are too overstretched as there are a lot of edge cases that will make things unnecessarily complex and may even confuse users.

Overriding Token component is straight forward. But what if the user wants to use Token override components with SwapAmountInput?

Few potential approaches are:

Composable SwapAmountInput component API like the top level Wallet and Swap component.

Use hooks + context to provide enough information for the user to build SwapAmountInput from the ground up.

I don't see what's the purpose of using SwapAmountInput with TokenSelect. I feel like we are trying to give too much freedom to users. And as long as React Context is used, components should be scoped. The following will not work without wrapping it inside its provider.

https://github.com/coinbase/onchainkit/blob/c099cc66e0909b89801e765b3dd1d40004ba0e07/src/swap/components/SwapAmountInput.tsx#L16-L24

Scenario 2 - Override everything

That's fine to some extent. The main issue is the level of complexity to re-implement a given feature. If it's too complex, people will give up.

Other than the name rotation: TokenSelectDropdown should use select and option instead of using div and button. Specifically, TokenSelect needs to be select and TokenSelectDropdown needs to use option

And beyond just using the correct HTML tags, accessibility includes aria attributes, focus, keyboard navigation, screen reader etc... For reference, the Select component from Radix is ~1500 lines without comments. If I were to write my own components, those are not things I would want to handle and it is actually backward when consuming a component library.

Before giving complete freedom to users, it would be best for OnchainKit components to be fully functional.

Taking TokenSelect as an example, besides not being accessible

It doesn't support the typeahead feature which is very common in Select components.
Given the number of tokens, sometimes it's more beneficial to search tokens asynchronously by fetching them from a bucket or whatever.
There's no way to group items (by chains for example).
There's no way to position where the select options are displayed (top, bottom) so it may go off the screen.

And then expand the scope once things are ironed-out.

Before giving complete freedom to users, it would be best for OnchainKit components to be fully functional.

Absolutely agreed there and that's what OCK wants and needs to be.

Taking TokenSelect as an example, besides not being accessible ...

Agreed there too. OCK approach has been to keep things simple then build it up. Team is very well aware of short comings of current components including full accessibility support.

That's fine to some extent. The main issue is the level of complexity to re-implement a given feature. If it's too complex, people will give up.

Absolutely. I do think that OCK can achieve this without being too difficult and actually be straight forward with some good documentation that is copy-paste friendly.

The following will not work without wrapping it inside its provider.

Agreed! OCK does not support UI library override in its current state. It's gonna take some work.

Let's breakdown Radix though...

Layout and Typography is unnecessary. OCK can do all this with tailwindcss + theme
Many Components can be done via tailwindcss pretty easily and many OCK has no use case for at the moment.

Keep things simple. While radix can solve the TokenSelect accessibility story, I'd be careful with doing a full integration.

At the end of the day, accessibility is the core issue here. Whether to solve that with Radix or not... I wouldn't purely give up on achieving that just because a UI library does it for you.

I am taking a long hiatus soon and wanted to give you, and the rest of the team, insight on where my head was at while I wrote up those components.

That is all to say, I won't be part of making the decision on how to address the accessibility or on revisiting the decision on Radix integration. I did plan out architecturally how to move forward but I expect things to change given that I won't be here to see it to the end.

Lastly, been loving your thoughtful and insightful contributions and questions.

Onchain team has been rocking it and hope they continue on with their good work.

It's been a pleasure to be part of this journey.

Thanks for all your insight and stewardship. There's definitely a lot to learn from that.

Cheers @kyhyco

coinbase / onchainkit