feat(components): Allow Fully Custom Combobox Activator

[ZakaryH]

Motivations

Some designs call for even further customization beyond a Chip or Button.

To enable this, we are going to open up the Activator even further to allow more customization.

Changes

Added

Combobox Activator can accept custom markup. No longer limited to only Chip and Button.

Changed

toggleOpen & handleOpen regardless of if we want to expose the API through render props or not, calling setOpen directly can be risky. setOpen(true) is fine. setOpen(false) is not since we want to make sure to do all the cleanup and invoke all the callbacks in addition to closing the combobox.

to that end, I've created an equivalent wrapper for setOpen(true): handleOpen. if we ever want to add callbacks for onOpen then it will be easy to add it to one place over all the places we were using setOpen(true). right now it's very bare bones.

by not making the setter directly available from the Context, or even exporting it from where we make the handlers, it is much harder to accidentally use it which we almost did with a recent change. additionally, if we end up exposing methods via render props it's much safer to expose the handleOpen

I went one step further and made a toggleOpen method that makes it even simpler where we call the toggle. rather than having the logic in a couple places, you just call this method. I could potentially be convinced to go back to handleClose/handleOpen. it's not a ton of logic to repeat and does offer more granular control, though I'm not too sure what you'd do any differently with it. you can only go back and forth between the states. calling open while it's open, or closed while it's closed make no sense.

Deprecated

Removed

Fixed

Security

Testing

Changes can be tested via Pre-release

---

In Atlantis we use Github's built in pull request reviews.

[cloudflare-workers-and-pages[bot]]

Deploying atlantis with    Cloudflare Pages

Latest commit:	2e8b0a6
Status:	✅  Deploy successful!
Preview URL:	https://e74cc3e5.atlantis.pages.dev
Branch Preview URL:	https://job-107039-custom-activator.atlantis.pages.dev

View logs

[ZakaryH]

@chris-at-jobber added a webstory with a div wrapping a h2 and an Icon for you to check out

about the "semantically a button" - we're going to stomp on the role provided and make it a combobox. not sure if that's exactly what you meant by that or something else entirely.

[chris-at-jobber]

we're going to stomp on the role provided and make it a combobox

Ah right we're handling that for the consumer now. That works!

[chris-at-jobber]

The story looks good, helps validate that you can pass in something truly custom. In terms of things like keyboard operability, is there anything we can/should do to automatically ensure the element receives keyboard focus, or is that something we could rely on consumers to handle? (ie always pass in a tabindex, or documenting that regardless of role the trigger element should use an html button so it gets the benefits of operability?)

[ZakaryH]

Ah right we're handling that for the consumer now. That works!

yeah! at least I'd like to. I can still be convinced of doing it another way, but this makes things quite simple for a consumer which I like.

[ZakaryH]

The story looks good, helps validate that you can pass in something truly custom. In terms of things like keyboard operability, is there anything we can/should do to automatically ensure the element receives keyboard focus, or is that something we could rely on consumers to handle? (ie always pass in a tabindex, or documenting that regardless of role the trigger element should use an html button so it gets the benefits of operability?)

good question

we can for sure also automatically give it tabindex=0 incase it's not a focusable element like a div

though, in that case we might also need to define an onKeydown to handle return and spacebar presses since we won't get the nice benefit of a button where you only have to define the onClick. if we don't explicitly define that for onKeydown you'd only be able to click on the div to interact with it

it's pretty trivial to add. it's just weird in the case where we're given a button which has no need for those extra definitions. as far as I'm aware though it doesn't hurt.

[ZakaryH]

after implementing both approaches I'm torn.

on one hand, I think this is a vastly superior consumer experience when all you want to do is modify the content of the activator button. not to mention, it might ensure a better end user experience since there's no opportunity to miss implementing the extra stuff.

      <Combobox.Activator>
          <div
            tabIndex={0}
            style={{
              display: "inline-flex",
              alignItems: "center",
              gap: "10px",
            }}
          >
            <Heading level={2}>Heading Two</Heading>
            <Icon name={"arrowDown"} />
          </div>
      </Combobox.Activator>

which is achieved via ComboboxActivator.tsx

``` return (

{ if (event.key === "Enter" || event.key === " ") { toggleOpen(); } }} > {props.children}

); ```

compared to

      <Combobox.Activator>
          {activatorAPI => (
            <div
              role={activatorAPI.role}
              tabIndex={0}
              aria-controls={activatorAPI.ariaControls}
              aria-expanded={activatorAPI.ariaExpanded}
              onClick={activatorAPI.toggleOpen}
              onKeyDown={e => {
                if (e.key === "Enter" || e.key === " ") {
                  activatorAPI.toggleOpen();
                }
              }}
              style={{
                display: "inline-flex",
                alignItems: "center",
                gap: "10px",
              }}
            >
              <Heading level={2}>Heading Two</Heading>
              <Icon name={"arrowDown"} />
            </div>
          )}
      </Combobox.Activator>

tabindex, focus & keyboard navigation we could even move the tabindex=0 into the div we're wrapping everything in for the first case. though there are some cases where that can get weird. if you provide a button, input or really any element where you want that thing to be the activator and it supports focus, then you'd have to tab twice to get to it since the wrapping div is focusable and the provided element is the first child. that's specifically if you want that top level item to be the activator.

if you provide a layout where there's another button nested inside like a chip suffix to clear the selection or something, that's fine to have to tab twice because it's a distinct other interactive element.

I don't know if that's something people would want to do? provide a button that wraps a layout? honestly that seems redundant. the wrapping div we've provided is acting as the button. we have a firm opinion that the first element needs to be button-like and the activator.

I'm trying to imagine a case where you want the activator to be a piece inside of the activator, not the wrapping element. I don't think that makes sense.

in this case, if someone wanted to make the OPEN the focusable element that would trigger you wouldn't be able to. however I don't think this is a good design. your clear button has no need to be in the activator. just make it a separate element, you have access to the state/ability to clear it. there's no reason to put it.

input

input is the exception to all this. giving the div a tabindex makes that flow bad. whereas exposing all of the activator API would enable someone to make a much better experience.

the downside is the above code. you have to put it all together yourself.

final thought

to me it comes down to what we want to optimize for. do we want to optimize for what is essentially custom button layouts, where it's easy and we take care of most of it for you, with an emphasis on accessibility/keyboard features.

or do we want to optimize for full customizability, where you build whatever you want but it's more complicated, you need to read the instructions and it's possible to miss pieces that are required. you'd be able to make a somewhat decent input experience.

the complicated way enables almost anything someone could think of. the simple way enables everything we've seen so far for custom Activator designs.

[MichaelParadis]

RE: This comment regarding the children approach. I feel that the current approach is preferable, while it is a bit more work for consumers and they need to read documentation about how to use it I feel that because this is for advanced usages, we should expect developers to handle the various edgecases that may arise (such as keyboard navigation, deciding when to open the Combobox, etc.). An example of this was when we decided to autocreate the sort labels on DataList but that ended up causing issues for us PR here.

the simple way enables everything we've seen so far for custom Activator designs.

and while this is true right now, I think doing "magic" to get it working with limited configuration is likely to lead to developers having issues implementing new custom activator designs in the future (as our design language evolves)

[github-actions[bot]]

Published Pre-release for 2e8b0a61bd6715260e195d7007c9cf40465061af with versions:

  - @jobber/components@5.43.1-JOB-107039-2e8b0a6.11+2e8b0a61

To install the new version(s) for Web run:

npm install @jobber/components@5.43.1-JOB-107039-2e8b0a6.11+2e8b0a61