fix(SortBy): Adds id prop to SortBy, Select components

[joshpensky]

Summary

This PR adds an id prop to the SortBy component, in order to associate the element with a label. To accomplish this, the PR also adds an id prop to the Select component

Result

1. Ran npm link in packages/react-instantsearch-dom and root directory to sync package update
   • Is there a better way to do this? yarn watch wasn't updating the component from the package
2. Added an id to the default SortBy Storybook story
3. Confirmed the id prop appears in DOM

Questions

• Where do we update the documentation for SortBy on the Algolia website to include the new id prop?

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1a917c5:

Sandbox	Source
react-instantsearch-app	Configuration
routing-basic	Configuration

[algobot]

✔️ Deploy Preview for react-instantsearch ready!

🔨 Explore the source changes: 573e698

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-instantsearch/deploys/60dc96c8a074c00007b57a8a

😎 Browse the preview: https://deploy-preview-3068--react-instantsearch.netlify.app/storybook/iframe

[Haroenv]

Thanks for your PR! I think this makes sense, since these controls otherwise are missing a label, but have a couple things that could be fixed before releasing:

• adding a test to the selector & sort by component around id being correctly forwarded
• doing the same for hitsPerPage component (I don't think there's other components, although arguably MenuSelect can also be done)

As an alternative before releasing, you could wrap the SortBy in a label, and then no id is needed (this is how I usually fix it, as ids have to be globally unique, which isn't something you can guarantee with a component-based framework)

[joshpensky]

Thank you for reviewing @Haroenv! 🙌

• adding a test to the selector & sort by component around id being correctly forwarded

I added a test for the SortBy component, but didn't see one for Select. Should I create a test file for that component?

• doing the same for hitsPerPage component (I don't think there's other components, although arguably MenuSelect can also be done)

I was planning on opening a few more PRs to add this id prop to different components, namely HitsPerPage and SearchBox (for the input element)! We can also add MenuSelect to this list

As an alternative before releasing, you could wrap the SortBy in a label, and then no id is needed (this is how I usually fix it, as ids have to be globally unique, which isn't something you can guarantee with a component-based framework)

This is something my team and I debated a bit as well when we made this PR! We thought adding the id prop would give the user more control over the markup of their search form

[Haroenv]

This seems good to go for me!

[Haroenv]

I'd like to release the three locations of ids in a single release, are you interested in making similar PRs for the other components? For SearchBox the name should probably be inputId instead of id though, since the root is a form, not an input.

Thanks!

[joshpensky]

Perfect, yes! I can push up those remaining PRs tomorrow

[joshpensky]

Hey @Haroenv! The PRs linked above (#3072, #3073, #3074) should fix the other discussed components for your next release 🙌

[Haroenv]

this style of test looks good for the other two as well 👌