lib: Support headers with SimpleSelect #21480
Conversation
| <SelectOption key={option.key} | ||
| isDisabled | ||
| className="ct-select-header"> | ||
| {option.content} | ||
| </SelectOption> |
There was a problem hiding this comment.
These 5 added lines are not executed by any test.
|
I start to think we should just use SelectOption et al with SimpleSelect instead of the extra indirection via the In my defense, I just went with the template offered by PatternFly without questioning it... |
There was a problem hiding this comment.
Putting SelectOptions into selects and such sounds good, as long as it's consistent (wrt. TypeaheadSelect as well). I'll let you decide if you want to go ahead with this PR (it looks nice, and doesn't really make the situation worse) to unblock the c-machines porting, or clean this up with SelectOption.
Thanks!
I think we should keep TypeaheadSelect as it is. The explicit data structure (and its restriction to be a flat list) is very helpful for the generic filtering and key navigation. PF is improving key navigation and we might not need to implement it ourselves anymore, and we might move the filtering into the clients of TypeaheadSelect (so that the client only creates the list of SelectOptions after filtering). But all PF examples for typeahead keep the custom key navigation, and it looks like the typeahead widget is supposed to keep the focus always on the text input, so that the user can type into it at all times, even in the middle of navigating the offered choices. But this feels like more change/work than I want to take on right now. So, hmm. This continues my general feeling about this work. Too many dilemmas and not enough help from PF. Let's land this and stumble forward. |
Needed to replace the deprecated select in cockpit-machines.