Re-approach the design of the PF5 <Select> component

[KKoukiou]

In cockpit we decided on an early adoption of PF5 (https://cockpit-project.org/blog/patternfly-v5-alpha.html) and I am coming back here with some feedback.

The topic here will be the new <Select> component. All in all I find it a bit unnecessarily complicated as exposed to the consumers. Let's enumerate the parts that could be IMO simplified.

• SelectOption: right now itemId and children are required properties, otherwise the component will not render. This is asking the user to pass the same value twice for most of the simple cases. Additionally itemId feels like 'id', html dom element identifier. The old property name 'value' is more exact to what this is.

• MenuToggle component can be hidden from the consumers in the simple cases at least. Compared to the previous implementation which hided the MenuToggle from the users, now the users of this components need to write much more boilerplate code.

This includes:
Two places to define if the select menu is open:

•  MenuToggle isExpanded={isOpen}
  

•  Select isOpen={isOpen}
  

Three places to control the open state of the component

•  MenuToggle onClick={() => setIsopen(!isOpen)}   // Cotrols events that change the open state from the toggle itself
  

•  Select onSelect={onSelect} // Controls events that change the open state when clicking on options
  

•  onOpenChange={isOpen => setIsOpen(isOpen)}  // Controls events that change the open state externally, for example clicking outside of the component or pressing Esc
  

Let me know if you 're open to re-design this for a better consumer experience.

CC @nicolethoen @tlabaj

[kmcfaul]

Regarding itemId, I do believe we can improve the functionality for basic use cases (setting the itemId or children when only one of the two is passed in by default). The name of the prop I think is because it used generically in Menu for all callbacks, and both Select and Dropdown, built off of Menu, matched props for consistency. @tlabaj What do you think about renaming this prop just for Select?

My largest concern with baking in the toggle and other interactions is that the new Select will run into the same situation as the previous implementation, which became fragile and difficult to update cleanly as it attempted to handle numerous disparate designs and edge cases. Even for simpler designs, there can still be a wide variety of customization for a toggle that would have to be exposed through properties (placeholder text, badges/chips, clear button, event handlers, etc) which will make the Select component more complicated/weighted. I'm not sure if it would be that much less complex with props than building a toggle, and would be far less flexible. What are you thinking for simple use cases? Single select only?

I think it would be ideal to have one place to apply isOpen. It may be slightly tricky as the state is controlled by the user and not the component, but theoretically we could clone the toggle after it is passed to the Select to pass the prop along internally. I would have to test this locally because I'm not sure if that method would interfere with the reference on the toggle though.

The MenuToggle onClick is the analog to the previous implementation's onToggle callback. onOpenChange is the only newly added handler for open/closed state manipulation, that opts-in to the built in handling that transfers focus from the toggle to the menu and allows clicks and keys to toggle the menu. It's a callback and not a flag because the component isn't responsible for the state. There's not really a way around it unfortunately.

[jpuzz0]

My thoughts on the renaming of the prop itemId to value is that it aligns with what many are used to using with HTML's native option tag. Having tailored prop names to these interpretations of Menu components would make sense to me. If we're willing to rename Menu based components to mimic a Select input, I think we should be open to renaming Menu based props as well for this Select.

My vote would also be to commit to MenuToggle as the default toggle, but maybe still expose the prop to allow for overrides. Defaulting to this makes sense since all other Select components in-play also are "Menu" components under the hood (Also, the documentation lives with the rest of the Menu components).

There's a lot that goes into a typeahead MenuToggle it seems, and it's a lot to remember (e.g. MenuToggle, TextInputGroup, TextInputGroupMain, TextInputGroupUtilities). I don't want us move in the direction of less flexibility though as some might prefer this extent of customization, and it could lead to another monolithic component as you've mentioned here already.

I think we can look into defaulting to certain behavior/components and allowing for overrides or look into making an extension for Select (as @wise-king-sullyman mentioned here). I do think though that an extension should not be limited to Select and instead should be a place where Form and other Form input related extensions could live.

[wise-king-sullyman]

One thought: we could have an extension to offer a more "out of the box functional" Select. We could even have the extension be a toolbox of different selects for the most common use cases to keep each one simpler and hopefully avoid ever getting into another 1,500 line component situation.

[nicolethoen]

(I believe) most products build their own Select component wrapper for this sort of reason. So this idea makes sense. Mostly i'm just wondering who maintains this and when do we find time?

[wise-king-sullyman]

I think we could actually have docs content from multiple packages show under one extensions nav item, so I think the ideal scenario would be that each product team that wants a custom select contributes theirs upstream and owns it, and we could have them all display on the site as one "community Selects" extension.

[nicolethoen]

Some of the PF team dug into this conversation a little deeper again today.
We are generally hesitant to bake too much business logic into the newer implementation of the Select component, because of the sheer volume of disparate use cases the component is often used to address.

We discussed a few different possible ways to aid in reducing boilerplate.

1. We will be taking a look at addressing [Select] The new typeahead select implementation is not hiding boilerplate code at all patternfly-react#9511 which requests attempting to reduce the volume of keyboard handling expected to be implemented by the consumer.
2. We will bring this discussion to our next office hours for discussion with the community. PatternFly could likely control the open/closed state of the Select/Dropdown optionally as a way to reduce boilerplate code needed as well.
3. Generally, since so many dev teams have different expectations and use common use cases for the select component, the PF team is already aware that many codebases do include an abstracted wrapper they use to consolidate their boilerplate so it only needs to exist in one place in the code. If that's something any dev team wants to discuss with the PF team, we'd be happy to provide interaction guidelines, accessibility assessments, and code reviews of any abstractions of the Select/Dropdown components. It's also possible that some of the abstractions written by product teams are something which could be made more shareable with other teams. If you have a Select/Dropdown abstraction or wrapper you think worth sharing, please reach out!

Ultimately, PF is not planning to provide abstractions ourselves because of the possible volume of requests and ongoing future maintenance that could create for which we don't have the bandwidth. But we are open to supporting any team as they try to build out PF Selects, Dropdowns or any menu like components!