Disable keyboard interactions for DropdownCore if component is disabled #2250

beaesguerra · 2024-06-11T22:19:11Z

Summary:

Remove storybook controls for SingleSelect disabled story. Because the storybook args included opened:false, the select was always closed, which was hiding a bug where the disabled select could still be opened using keyboard arrows
Add disabled prop to DropdownCore so that it's interactions can be disabled if it is true. This is needed now that the SelectOpener does not have the disabled attribute set (since we use aria-disabled now so that it can be focusable)
Add tests to make sure event handlers aren't called when a component is disabled
Prevent Single and Multi Select from being in an open state when the opened prop and disabled prop are both true

Test plan:

Confirm that the SingleSelect and MultiSelect cannot be opened when disabled, especially by using the down arrow key to open the listbox
- ?path=/story/packages-dropdown-singleselect--disabled
- ?path=/story/packages-dropdown-multiselect--disabled

…e storybook args included opened:false, the select was always closed, which was hiding a bug where the disabled select could still be opened using keyboard arrows

…sabled if it is true. This is needed now that the dropdown opener could potentially not have the disabled attribute set (since we use aria-disabled now on the selectopener so that it can be focusable

changeset-bot · 2024-06-11T22:19:15Z

🦋 Changeset detected

Latest commit: d266079

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@khanacademy/wonder-blocks-dropdown	Patch
@khanacademy/wonder-blocks-birthday-picker	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-06-11T22:21:08Z

Size Change: +15 B (+0.02%)

Total Size: 96.8 kB

Filename	Size	Change
`packages/wonder-blocks-dropdown/dist/es/index.js`	14.1 kB	+15 B (+0.11%)

ℹ️ View Unchanged

Filename	Size
`packages/wonder-blocks-accordion/dist/es/index.js`	3.78 kB
`packages/wonder-blocks-banner/dist/es/index.js`	1.52 kB
`packages/wonder-blocks-birthday-picker/dist/es/index.js`	1.72 kB
`packages/wonder-blocks-breadcrumbs/dist/es/index.js`	1.13 kB
`packages/wonder-blocks-button/dist/es/index.js`	4.28 kB
`packages/wonder-blocks-cell/dist/es/index.js`	2.24 kB
`packages/wonder-blocks-clickable/dist/es/index.js`	3.3 kB
`packages/wonder-blocks-core/dist/es/index.js`	3.66 kB
`packages/wonder-blocks-data/dist/es/index.js`	6.34 kB
`packages/wonder-blocks-form/dist/es/index.js`	5.35 kB
`packages/wonder-blocks-grid/dist/es/index.js`	1.36 kB
`packages/wonder-blocks-i18n/dist/es/index.js`	4.6 kB
`packages/wonder-blocks-icon-button/dist/es/index.js`	3.21 kB
`packages/wonder-blocks-icon/dist/es/index.js`	1.06 kB
`packages/wonder-blocks-labeled-field/dist/es/index.js`	72 B
`packages/wonder-blocks-layout/dist/es/index.js`	1.94 kB
`packages/wonder-blocks-link/dist/es/index.js`	2.53 kB
`packages/wonder-blocks-modal/dist/es/index.js`	5.51 kB
`packages/wonder-blocks-pill/dist/es/index.js`	1.64 kB
`packages/wonder-blocks-popover/dist/es/index.js`	4.85 kB
`packages/wonder-blocks-progress-spinner/dist/es/index.js`	1.52 kB
`packages/wonder-blocks-search-field/dist/es/index.js`	1.54 kB
`packages/wonder-blocks-switch/dist/es/index.js`	2.09 kB
`packages/wonder-blocks-testing/dist/es/index.js`	3.94 kB
`packages/wonder-blocks-theming/dist/es/index.js`	693 B
`packages/wonder-blocks-timing/dist/es/index.js`	1.8 kB
`packages/wonder-blocks-tokens/dist/es/index.js`	1.74 kB
`packages/wonder-blocks-toolbar/dist/es/index.js`	855 B
`packages/wonder-blocks-tooltip/dist/es/index.js`	6.91 kB
`packages/wonder-blocks-typography/dist/es/index.js`	1.48 kB

_{compressed-size-action}

codecov · 2024-06-11T22:22:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.84%. Comparing base (308fe79) to head (d266079).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2250      +/-   ##
==========================================
+ Coverage   94.02%   95.84%   +1.81%     
==========================================
  Files         248      248              
  Lines       29362    29380      +18     
  Branches     1639     1811     +172     
==========================================
+ Hits        27609    28158     +549     
+ Misses       1749     1208     -541     
- Partials        4       14      +10

Files	Coverage Δ
...r-blocks-dropdown/src/components/dropdown-core.tsx	`90.86% <100.00%> (-0.86%)`	⬇️
...er-blocks-dropdown/src/components/multi-select.tsx	`99.83% <100.00%> (+<0.01%)`	⬆️
...r-blocks-dropdown/src/components/single-select.tsx	`100.00% <100.00%> (ø)`

... and 47 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 308fe79...d266079. Read the comment docs.

github-actions · 2024-06-11T22:22:07Z

A new build was pushed to Chromatic! 🚀

undefined

Chromatic results:

Metric	Total
Captured snapshots	undefined
Tests with visual changes	undefined
Total stories	undefined
Inherited (not captured) snapshots [TurboSnap]	undefined
Tests on the build	undefined

…is disabled

khan-actions-bot · 2024-06-11T22:31:00Z

Gerald

Required Reviewers

@Khan/wonder-blocks for changes to .changeset/curvy-frogs-listen.md, __docs__/wonder-blocks-dropdown/single-select.stories.tsx, packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx, packages/wonder-blocks-dropdown/src/components/multi-select.tsx, packages/wonder-blocks-dropdown/src/components/single-select.tsx, packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx, packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx, packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx, packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

github-actions · 2024-06-11T22:32:46Z

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (a008181) and published all packages with changesets to npm.

You can install the packages in webapp by running:

./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2250"

Packages can also be installed manually by running:

yarn add @khanacademy/wonder-blocks-<package-name>@PR2250

jandrade · 2024-06-12T14:52:30Z

packages/wonder-blocks-dropdown/src/components/multi-select.tsx

suggestion: This makes me realize that we should force the listbox/dropdown to be closed when it is disabled even if open/opened is set to true.

Right now, if I use the Default story and turn on both the opened and disabled controls, then the listbox will be shown, but to me it looks incorrect as disabled should take more precedence (IMO). What are your thoughts on that?

https://5e1bf4b385e3fb0020b7073c-nhmqyuvdyi.chromatic.com/iframe.html?args=opened:!true;disabled:!true&id=packages-dropdown-singleselect--default&viewMode=story

That's a good point! I've updated it so the derived open state will always be false if the disabled prop is true!

…d prop is true

jandrade

LGTM! Thanks for fixing this

jandrade · 2024-06-12T18:28:02Z

packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx

praise: Thanks for adding all the tests (including the open + disabled case.

beaesguerra added 2 commits June 11, 2024 15:20

Remove storybook controls for SingleSelect disabled story. Because th…

0fbd580

…e storybook args included opened:false, the select was always closed, which was hiding a bug where the disabled select could still be opened using keyboard arrows

Add disabled prop to DropdownCore so that it's interactions can be di…

a1957bc

…sabled if it is true. This is needed now that the dropdown opener could potentially not have the disabled attribute set (since we use aria-disabled now on the selectopener so that it can be focusable

beaesguerra self-assigned this Jun 11, 2024

beaesguerra added 2 commits June 11, 2024 16:24

Add tests to make sure event handlers aren't called when a component …

22bbb14

…is disabled

Add changeset

9188844

beaesguerra force-pushed the fix-dropdown-interaction-when-disabled branch from 39c5f11 to 9188844 Compare June 11, 2024 22:24

beaesguerra marked this pull request as ready for review June 11, 2024 22:30

khan-actions-bot requested a review from a team June 11, 2024 22:31

jandrade reviewed Jun 12, 2024

View reviewed changes

beaesguerra added 2 commits June 12, 2024 09:39

Prevent single/multiselect from being in an open state if the disable…

1c33005

…d prop is true

Merge branch 'main' into fix-dropdown-interaction-when-disabled

d266079

beaesguerra requested a review from jandrade June 12, 2024 15:44

jandrade approved these changes Jun 12, 2024

View reviewed changes

beaesguerra merged commit f7ff9a7 into main Jun 12, 2024
25 of 27 checks passed

beaesguerra deleted the fix-dropdown-interaction-when-disabled branch June 12, 2024 19:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable keyboard interactions for DropdownCore if component is disabled #2250

Disable keyboard interactions for DropdownCore if component is disabled #2250

beaesguerra commented Jun 11, 2024 •

edited

Loading

changeset-bot bot commented Jun 11, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading

codecov bot commented Jun 11, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading

khan-actions-bot commented Jun 11, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading

jandrade Jun 12, 2024

beaesguerra Jun 12, 2024

jandrade left a comment

jandrade Jun 12, 2024

Disable keyboard interactions for DropdownCore if component is disabled #2250

Disable keyboard interactions for DropdownCore if component is disabled #2250

Conversation

beaesguerra commented Jun 11, 2024 • edited Loading

Summary:

Test plan:

changeset-bot bot commented Jun 11, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Jun 11, 2024 • edited Loading

codecov bot commented Jun 11, 2024 • edited Loading

Codecov Report

github-actions bot commented Jun 11, 2024 • edited Loading

A new build was pushed to Chromatic! 🚀

Chromatic results:

khan-actions-bot commented Jun 11, 2024 • edited Loading

Gerald

github-actions bot commented Jun 11, 2024 • edited Loading

npm Snapshot: Published

jandrade Jun 12, 2024

Choose a reason for hiding this comment

beaesguerra Jun 12, 2024

Choose a reason for hiding this comment

jandrade left a comment

Choose a reason for hiding this comment

jandrade Jun 12, 2024

Choose a reason for hiding this comment

beaesguerra commented Jun 11, 2024 •

edited

Loading

changeset-bot bot commented Jun 11, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading

codecov bot commented Jun 11, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading

khan-actions-bot commented Jun 11, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading