[Autocomplete.Combobox] Convert to functional component

[genevieveluyt]

WHY are these changes introduced?

The Autocomplete component causes the following warning if used with server-side rendering:

Warning: Prop `aria-controls` did not match. Server: "ComboBox2" Client: "ComboBox1"

This is a known problem: #1761

Although this PR does not directly fix that problem, converting the Combobox to a functional component and having it use the useUniqueId hook is a prerequisite for fixing the automatic ID generation in the future.

WHAT is this pull request doing?

Converts the Autocomplete Combobox component into a functional component and uses the useUniqueId hook.

How to 🎩

Not easily testable without server side rendering 🤔

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:

import React, {useState} from 'react';

import {Page, Autocomplete} from '../src';

export function Playground() {
  const [input, setInput] = useState('');
  const [selected, setSelected] = useState<string[]>([]);

  const textFieldMarkup = (
    <Autocomplete.TextField
      label="Textfield"
      value={input}
      onChange={setInput}
    />
  );
  const options = ['one', 'two', 'three'].map((item) => {
    return {value: item, label: item};
  });
  return (
    <Page title="Playground">
      <Autocomplete
        options={options}
        selected={selected}
        onSelect={setSelected}
        textField={textFieldMarkup}
        allowMultiple
      />
    </Page>
  );
}

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated the component's README.md with documentation changes
• Tophatted documentation changes in the style guide
• For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

[ghost]

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

[github-actions]

🟢 This pull request modifies 3 files and might impact 1 other files.

Details:

All files potentially affected (total: 1)

• /src/components/Autocomplete/Autocomplete.tsx

📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Autocomplete/components/ComboBox/ComboBox.tsx (total: 1)

Files potentially affected (total: 1)

• /src/components/Autocomplete/Autocomplete.tsx

🧩 src/components/Autocomplete/components/ComboBox/tests/ComboBox.test.tsx (total: 0)

Files potentially affected (total: 0)

[BPScott]

Thanks for taking this on @genevieveluyt!

I've not ran this just yet to test behaviour, but I've spotted some simplifications that could be done

[BPScott]

As above, though tbh I'm not very clear on why we need to track the previous value of the popover state

If we do indeed still need this then it seems like an idea case for a usePrevious utility hook. We don't currently depend upon that @shopify/react-hooks and instead roll our own helpers, so copying that hook into our utilities folder at src/utilities/use-previous.ts (and its test at `src/utilities/use-previous.test.ts) would be the right place to put it. That pattern matches our other utility hooks like useToggle.

usePrevious should save a bunch of setPopoverActive(false); setPopoverWasActive(false); code, as we can only talk about setting popoverActive, and not worry about popoverWasActive

[genevieveluyt]

I removed the previous state and it didn't break any tests. I'm not sure if that logic was there for a specific case not covered by tests though 🤔

[BPScott]

A few more little tidyings that became apparent since the prior simplifications

[BPScott]

Just gave this a click around in storybook and seems good to me. I'd appreciate some extra eyes though as I'm not that familiar with the Comobox's behaviour. @dfmcphee / @chloerice would you be able to take some time next week to give this a review too?

[BPScott]

Oh and a little expectation setting: This PR alone won't solve the automatic id mismatches between server and client, but the converting it to a functional component is a prerequisite of that future fix, which will be inside useUniqueId() - either through adding functionality ourselves or by leveraging useOpaqueIdentifier()

[genevieveluyt]

Ah ok I misunderstood. That means I'll still need to set the ID manually for the time being?

[chloerice]

Hey @genevieveluyt, thanks for working on this! I'll tophat/review this by EOD Wednesday 😊

[chloerice]

Hey @genevieveluyt, 🎩 looks good 👍(any issues I found pre-exist these changes) Thank you for contributing ❤️!!!

You'll just need to add an entry to UNRELEASED.md under Code quality to pass the change log check

- Converted `ComboBox` to a functional component ([#2918](https://github.com/Shopify/polaris-react/pull/2918))

[ghost]

🎉 Thanks for your contribution to Polaris React!

[BPScott]

Thanks @genevieveluyt!