Fix events fired by Select

[justinanastos]

Release Notes

The Select component needed to be refactored and this will include breaking API changes.

• Remove label prop from Select

  We had a label prop before to indicate what to populate the select with when there was no content; selects already have this logic by giving an option with an empty value: <option value="">empty text here</option>

• Rename defaultValue to initialValue to be more inline with other non-controlled React elements

• Add triggerAs prop and remove passthrough className to remove confusion as to where a top-level className would be used

• Add labelPropsCallbackRef prop.

  This is the magic of this PR. This will be called with an object of props to be spread onto your <label> component. It is called intelligently as to prevent infinite re-renders.

• Ensure this behaves correctly with formik by adding initial unit testing to actually use formik. This exposed the issue that we don't provide any onBlur callback, which I added here as well.

📦 Published PR as canary version: 8.3.2-canary.279.6749.0

✨ Test out this PR locally via:

npm install @apollo/space-kit@8.3.2-canary.279.6749.0
# or 
yarn add @apollo/space-kit@8.3.2-canary.279.6749.0

[timbotnik]

PR seems fine, but I must admit I don't immediately understand the use-case for this label / callback props thing. Can you point me to an example where we need this?

[justinanastos]

PR seems fine, but I must admit I don't immediately understand the use-case for this label / callback props thing. Can you point me to an example where we need this?

@timbotnik Great question. We're not using an actual <select> here so we have to somehow indicate that this is intended to function like a <select>. Downshift's useSelect handles that internally by providing a bunch of callbacks that will generate the props to put on the trigger (a Button in this case), the label because a <select> shouldn't be wrapped in a <label>, and the menu itself. These callbacks all include id, role, and aria-labelledby props that tie the elements together without us having to think about it.

That being said; this makes way more sense when we have a <label> rendered alongside the Button and Popover; but I didn't want to do that because I want to create a FormInput (or something like that) that will take the form element, like Select or TextInput (or CheckBox or RadioGroup) and then render the label, helper text, and validation. This requires us to have the <label> be rendered outside of the Select component.

This left me with a few choices:

• Expect the user to pass props for the trigger and menu with accessibility props, such as this:

  <label id="select-label-1" for="select-toggle-button-1">label</label>
  <Select
    triggerAs={<Button id="select-toggle-button-1" aria-labelledby="select-label-1" />}
    menuAs={<Popover id="select-menu-1" aria-labelledby="select-label-1" />}
  >
    ...
  </Select>

  This totally works but feels like we're asking a lot of the user. I'd ideally like this to just work.

• Use a prop that will generate predictable ids and aria-labelledby props that you will then also use for your own <label>

  <label id="select-1-label" for="select-1-toggle-button-1">label</label>
  <Select ariaName="select-1" >
    ...
  </Select>

  This works too, but it requires an implicit contract between the consumer and the component and requires the user to know about it, which, again, feels like a lot to ask.

• Create a new hook with a new hook that wraps useSelect to provide labelProps to the consumer.

  I didn't like this approach because I wanted the API to feel intuitive, not like yet another new language or API to learn.

• Lastly, I figured I could send these downshift-generated props back to the consumer to be rendered inside the label. That's what I did here.

  const [labelProps, setLabelProps] = React.useState();
  
  return (
    <>
      <label {...labelProps}>{labelText}</label>
      <Select initialValue="" labelPropsCallbackRef={setLabelProps}>
        <option value="">select an item</option>
        <option value="a">a</option>
        <option value="b">b</option>
      </Select>
    </>
  );

  I felt like this the best way to solve the problem. I know the labelPropsCallbackRef is weird, but it makes the most sense to me.

[justinanastos]

@timbotnik This also made me realize the storybook story was using the wrong prop name so the <label> wasn't actually being applied there 😃

[justinanastos]

@timbotnik I re-requested a review because I added a few more commits, specifically one to trickily add onBlur behavior.

[apollo-bot2]

🚀 PR was released in v8.5.0 🚀