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

feat(useCombobox): toggle menu on input click #1440

Merged
merged 5 commits into from
Jun 11, 2023

Conversation

silviuaavram
Copy link
Collaborator

@silviuaavram silviuaavram commented Nov 23, 2022

What:
useCombobox should toggle the menu on input click, as ARIA suggests. The previous implementation to open the menu on input focus was a bad interpretation of the combobox 1.2 a11y pattern, and this PRs aims to fix the behaviour.

The InputFocus state change will be kept so it won't produce a breaking change, but focusing input will perform no state change, it will not open the menu anymore, and the state change type will be removed in a subsequent release.

Why:
Fixes #1439.

How:
Remove the focus input behaviour of opening the menu.
Handle input click, make it behave as the toggle button click.
Keep useCombobox.stateChangeTypes.InputFocus for backwards compatibility, although it will not end up as type in the stateReducer and onChange props since focusing the input will not open the menu anymore.
The migration guide has been updated.

Checklist:

  • Documentation
  • Tests
  • TypeScript Types
  • Flow Types
  • Ready to be merged

@codecov-commenter
Copy link

codecov-commenter commented Nov 23, 2022

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (0d0bc32) 100.00% compared to head (e45fda4) 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff            @@
##                v8     #1440   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         1708      1712    +4     
  Branches       515       517    +2     
=========================================
+ Hits          1708      1712    +4     
Impacted Files Coverage Δ
src/hooks/useCombobox/index.js 100.00% <100.00%> (ø)
src/hooks/useCombobox/reducer.js 100.00% <100.00%> (ø)
src/hooks/useCombobox/testUtils.js 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@silviuaavram silviuaavram force-pushed the feat/useCombobox/toggle-on-input-click branch 2 times, most recently from 7241071 to e2553e0 Compare November 23, 2022 10:45
@silviuaavram silviuaavram changed the base branch from master to v8 June 11, 2023 09:43
@silviuaavram silviuaavram force-pushed the feat/useCombobox/toggle-on-input-click branch from 9087174 to 16b3f5f Compare June 11, 2023 10:12
@silviuaavram silviuaavram force-pushed the feat/useCombobox/toggle-on-input-click branch from 16b3f5f to e725c28 Compare June 11, 2023 11:21
@silviuaavram silviuaavram changed the base branch from v8 to master June 11, 2023 11:24
@silviuaavram silviuaavram changed the base branch from master to v8 June 11, 2023 11:24
@silviuaavram silviuaavram merged commit 7ab19e0 into v8 Jun 11, 2023
2 checks passed
@silviuaavram silviuaavram mentioned this pull request Jun 11, 2023
Merged
silviuaavram added a commit that referenced this pull request Jul 13, 2023
* feat(useCombobox): toggle menu on input click

* prepare for v8 instead of v7.1

* remove InputFocus mention from readme

* improve the migration guide

* remove forgotten comment
silviuaavram added a commit that referenced this pull request Jul 15, 2023
BREAKING CHANGE: Release Downshift v8.

## PRs included:
#1440
#1445
#1463
#1510
#1482

## Changes

These changes will also be covered in the [V8 migration guide](https://github.com/downshift-js/downshift/blob/master/src/hooks/MIGRATION_V8.md).

### isItemDisabled

Both `useCombobox` and `useSelect` now support the `isItemDisabled` function. This new API is used to mark menu items as disabled, and as such remove the from the navigation and prevent them from being selected. The old API required passing the `disabled` prop to the `getItemProps` function. This old API has been removed and you will receive a console warning if you are trying to use the `disabled` prop in getItemProps.

Example of API migration:

Old:

```jsx
const items = [{value: 'item1'}, {value: 'item2'}]

const {getInputProps, ...rest} = useCombobox({items})

return (
  // ... rest
  <li {...getItemProps({item, disabled: item.value === 'item2'})}>
)
```

New:

```jsx
const items = [{value: 'item1'}, {value: 'item2'}]

const {getInputProps, ...rest} = useCombobox({items, isItemDisabled(item, _index) { return item.value === 'item2' }})

return (
  // ... rest
  <li {...getItemProps({item})}>
)
```

The API for Downshift remains unchange.

### useCombobox input click

[ARIA 1.2](combobox-aria-example) recommends to toggle the menu open state at input click. Previously, in v7, the menu was opened on receiving focus, from both mouse and keyboard. Starting with v8, input focus will not trigger any state change anymore. Only the input click event will be handled and will trigger a menu toggle. Consequently:

- getInputProps **will not** return any _Focus_ event handler.
- getInputProps **will** return a _Click_ event handler.
- `useCombobox.stateChangeTypes.InputFocus` has been removed.
- `useCombobox.stateChangeTypes.InputClick` has been added instead.

We recommend having the default toggle on input click behaviour as it's part of the official ARIA combobox 1.2 spec, but if you wish to override it and not toggle the menu on click, use the stateReducer:

```js
function stateReducer(state, actionAndChanges) {
  const {changes, type} = actionAndChanges
  switch (type) {
    case useCombobox.stateChangeTypes.InputClick:
      return {
        ...changes,
        isOpen: state.isOpen, // do not toggle the menu when input is clicked.
      }
    default:
      return changes
  }
}
```

If you want to return to the v7 behaviour and open the menu on focus, keep the reducer above so you remove the toggle behaviour, and call the _openMenu_ imperative function, returned by useCombobox, in a _onFocus_ handler passed to
_getInputProps_:

```js
<input
  {...getInputProps({
    onFocus() {
      openMenu()
    },
  })}
/>
```

Consider to use the default 1.2 ARIA behaviour provided by default in order to align your widget to the accessibility official spec. This behaviour consistency improves the user experience, since all comboboxes should behave the same and
there won't be need for any additional guess work done by your users.

### Getter props return value types

Previously, the return value from the getter props returned by both Downshift and the hooks was `any`. In v8 we improved the types in order to reflect what is actually returned: the default values return by the getter prop function and
whatever else the user passes as arguments. The type changes are done in [this PR](#1482), make sure you adapt your TS code, if applicable.

Also, in the `Downshift` component, the return values for some getter prop values have changed from `null` to `undefined`, since that is what HTML elements expect (value or undefined). These values are also reflected in the TS types.

- getRootProps: 'aria-owns': isOpen ? this.menuId : ~~null~~undefined,
- getInputProps:
  - 'aria-controls': isOpen ? this.menuId : ~~null~~undefined
  - 'aria-activedescendant': isOpen && typeof highlightedIndex === 'number' &&
    highlightedIndex >= 0 ? this.getItemId(highlightedIndex) : ~~null~~undefined
- getMenuProps: props && props['aria-label'] ? ~~null~~undefined : this.labelId,

### environment propTypes

The `environment` prop is useful when you are using downshift in a context
different than regular DOM. Its TS type has been updated to contain `Node` and
the propTypes have also been updated to reflect the properties which are
required by downshift from `environment`.

[combobox-aria-example]:
  https://www.w3.org/WAI/ARIA/apg/example-index/combobox/combobox-autocomplete-list.html
robinandeer pushed a commit to HedvigInsurance/racoon that referenced this pull request Jul 25, 2023
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [downshift](https://downshift-js.com) ([source](https://togithub.com/downshift-js/downshift)) | [`7.6.2` -> `8.0.0`](https://renovatebot.com/diffs/npm/downshift/7.6.2/8.0.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/downshift/8.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/downshift/8.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/downshift/7.6.2/8.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/downshift/7.6.2/8.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>downshift-js/downshift (downshift)</summary>

### [`v8.0.0`](https://togithub.com/downshift-js/downshift/releases/tag/v8.0.0)

[Compare Source](https://togithub.com/downshift-js/downshift/compare/v7.6.2...v8.0.0)

##### Features

-   v8 ([#&#8203;1509](https://togithub.com/downshift-js/downshift/issues/1509)) ([b62fe05](https://togithub.com/downshift-js/downshift/commit/b62fe0518215570674ef45c06057475524f0ad4a))

##### BREAKING CHANGES

-   Release Downshift v8.

#### PRs included:

[downshift-js/downshift#1440
[downshift-js/downshift#1445
[downshift-js/downshift#1463
[downshift-js/downshift#1510
[downshift-js/downshift#1482

#### Changes

These changes will also be covered in the [V8 migration guide](https://togithub.com/downshift-js/downshift/blob/master/src/hooks/MIGRATION_V8.md).

##### isItemDisabled

Both `useCombobox` and `useSelect` now support the `isItemDisabled` function. This new API is used to mark menu items as disabled, and as such remove the from the navigation and prevent them from being selected. The old API required passing the `disabled` prop to the `getItemProps` function. This old API has been removed and you will receive a console warning if you are trying to use the `disabled` prop in getItemProps.

Example of API migration:

Old:

```jsx
const items = [{value: 'item1'}, {value: 'item2'}]

const {getInputProps, ...rest} = useCombobox({items})

return (
  // ... rest
  <li {...getItemProps({item, disabled: item.value === 'item2'})}>
)
```

New:

```jsx
const items = [{value: 'item1'}, {value: 'item2'}]

const {getInputProps, ...rest} = useCombobox({items, isItemDisabled(item, _index) { return item.value === 'item2' }})

return (
  // ... rest
  <li {...getItemProps({item})}>
)
```

The API for Downshift remains unchange.

##### useCombobox input click

[ARIA 1.2](combobox-aria-example) recommends to toggle the menu open state at input click. Previously, in v7, the menu was opened on receiving focus, from both mouse and keyboard. Starting with v8, input focus will not trigger any state change anymore. Only the input click event will be handled and will trigger a menu toggle. Consequently:

-   getInputProps **will not** return any *Focus* event handler.
-   getInputProps **will** return a *Click* event handler.
-   `useCombobox.stateChangeTypes.InputFocus` has been removed.
-   `useCombobox.stateChangeTypes.InputClick` has been added instead.

We recommend having the default toggle on input click behaviour as it's part of the official ARIA combobox 1.2 spec, but if you wish to override it and not toggle the menu on click, use the stateReducer:

```js
function stateReducer(state, actionAndChanges) {
  const {changes, type} = actionAndChanges
  switch (type) {
    case useCombobox.stateChangeTypes.InputClick:
      return {
        ...changes,
        isOpen: state.isOpen, // do not toggle the menu when input is clicked.
      }
    default:
      return changes
  }
}
```

If you want to return to the v7 behaviour and open the menu on focus, keep the reducer above so you remove the toggle behaviour, and call the *openMenu* imperative function, returned by useCombobox, in a *onFocus* handler passed to
*getInputProps*:

```js
<input
  {...getInputProps({
    onFocus() {
      openMenu()
    },
  })}
/>
```

Consider to use the default 1.2 ARIA behaviour provided by default in order to align your widget to the accessibility official spec. This behaviour consistency improves the user experience, since all comboboxes should behave the same and
there won't be need for any additional guess work done by your users.

##### Getter props return value types

Previously, the return value from the getter props returned by both Downshift and the hooks was `any`. In v8 we improved the types in order to reflect what is actually returned: the default values return by the getter prop function and
whatever else the user passes as arguments. The type changes are done in [this PR](https://togithub.com/downshift-js/downshift/pull/1482), make sure you adapt your TS code, if applicable.

Also, in the `Downshift` component, the return values for some getter prop values have changed from `null` to `undefined`, since that is what HTML elements expect (value or undefined). These values are also reflected in the TS types.

-   getRootProps: 'aria-owns': isOpen ? this.menuId : ~~null~~undefined,
-   getInputProps:
    -   'aria-controls': isOpen ? this.menuId : ~~null~~undefined
    -   'aria-activedescendant': isOpen && typeof highlightedIndex === 'number' &&
        highlightedIndex >= 0 ? this.getItemId(highlightedIndex) : ~~null~~undefined
-   getMenuProps: props && props\['aria-label'] ? ~~null~~undefined : this.labelId,

##### environment propTypes

The `environment` prop is useful when you are using downshift in a context
different than regular DOM. Its TS type has been updated to contain `Node` and
the propTypes have also been updated to reflect the properties which are
required by downshift from `environment`.

[combobox-aria-example]: https://www.w3.org/WAI/ARIA/apg/example-index/combobox/combobox-autocomplete-list.html

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday,before 5am every weekday,every weekend" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/HedvigInsurance/racoon).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMS4wIiwidXBkYXRlZEluVmVyIjoiMzYuMTEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feedback] New input focus behaviour added in v7.0.0
2 participants