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

CustomSelectControl : explore ariakit refactor #55234

Closed
wants to merge one commit into from

Conversation

 brookewp
Copy link
Contributor

 @brookewp brookewp commented Oct 11, 2023

What?

WIP

Tracked in #55023

Why?

How?

  • Review existing features of CustomSelectControl
  • Create a rough component to start exploring API
  • Determine if we can replace our current component
  • Assess current tests for CustomSelectControl
  • Add tests to the new component (cover any bugs, new features, etc)
  • Refine types, add docs, and stories
  • Request design input and implement styles
  • Look at requests for new component
    • Render children for more flexibility and customization

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

 @brookewp brookewp added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Oct 11, 2023
 @brookewp brookewp self-assigned this Oct 11, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more .

To note, this is just temporary CSS added from the Ariakit example.

const { label, onChange, options } = props;

const store = Ariakit.useSelectStore( {
setValue: ( value ) => onChange?. ( value ),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more .

Our current CustomSelectControl expects an object to be returned via the onChange event:

} = useSelect ( {
initialSelectedItem : items [ zero ] ,
items ,
itemToString ,
onSelectedItemChange ,
... ( typeof _selectedItem !== 'undefined' && _selectedItem !== null
? { selectedItem : _selectedItem }
: undefined ) ,

Which is managed by useSelect ; part of the package downshift .

In Ariakit, setValue appears to be what we'd use in lieu of useSelect , but value / setValue expect a string (or an array of strings for multi-select).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more .

It appears that the object passed to onSelectItemChange has the following structure:

 {
   selectedItem : "Curium" ,
   type : "__item_click__" ,
   highlightedIndex : - one ,
   isOpen : false ,
   inputValue : "" ,
 }

I'm not sure if all the elements in this structure are part of the public API. The type value, in particular, seems obscure. I'm also unsure about the meaning of highlightedIndex , as it always appears to be -1 .

Regardless, everything except for type can be readily accessed from within the setValue callback. You can use store.getState() in this context as well:

 const  store  =  useSelectStore ( {
   async  setValue ( selectedItem )  {
     if  ( ! onChange )  return ;
     // Executes the logic in a microtask after the popup is closed.  This might not be necessary.
     // This is simply to ensure the isOpen state matches that in Downshift.
     await  Promise . resolve ( ) ;
     const  state  =  store . getState ( ) ;
     const  changeObject  =  { selectedItem ,
       type : "__item_click__" ,
       highlightedIndex : state . renderedItems . findIndex ( ( item )  =>  item . value  ===  selectedItem ) ,
       isOpen : state . open ,
       inputValue : "" ,
     } ;
     onChange ( changeObject ) ;
   } ,
 } ) ;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more .

Thank you for this! We finally have gotten a chance to try this out. 🙂

 @brookewp
Copy link
Contributor Author

Going to close this as we have two more recent options for this: #57902 and #57000

Sign up for free to join this conversation on GitHub . Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
Status: Abandoned ⛔️
Development

Successfully merging this pull request may close these issues.

2 participants