fix(LEMS-2086): Add aria label to Popover Content #2263

anakaren-rojas · 2024-06-27T21:04:24Z

Summary:

Aria-labels are not being added to the View component, causing accessibility issues.
This PR adds aria-label to View

Issue: LEMS-2089

Test plan:

changeset-bot · 2024-06-27T21:04:27Z

🦋 Changeset detected

Latest commit: c4db32e

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

This PR includes changesets to release 1 package

Name	Type
@khanacademy/wonder-blocks-popover	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

khan-actions-bot · 2024-06-27T21:04:38Z

Gerald

Required Reviewers

@Khan/wonder-blocks for changes to .changeset/selfish-spiders-care.md, packages/wonder-blocks-popover/src/components/popover-content-core.tsx, packages/wonder-blocks-popover/src/components/__tests__/popover.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-27T21:06:18Z

Size Change: +6 B (+0.01%)

Total Size: 97.1 kB

Filename	Size	Change
`packages/wonder-blocks-popover/dist/es/index.js`	4.86 kB	+6 B (+0.12%)

ℹ️ View Unchanged

Filename	Size
`packages/wonder-blocks-accordion/dist/es/index.js`	3.78 kB
`packages/wonder-blocks-banner/dist/es/index.js`	1.53 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.25 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-dropdown/dist/es/index.js`	14.4 kB
`packages/wonder-blocks-form/dist/es/index.js`	5.34 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.23 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-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}

github-actions · 2024-06-27T21:07:38Z

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-homvxqunmu.chromatic.com/

Chromatic results:

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

codecov · 2024-06-27T21:08:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.76%. Comparing base (dcafa86) to head (c4db32e).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2263      +/-   ##
==========================================
+ Coverage   93.91%   95.76%   +1.85%     
==========================================
  Files         248      248              
  Lines       29514    29516       +2     
  Branches     2407     1847     -560     
==========================================
+ Hits        27717    28267     +550     
+ Misses       1793     1236     -557     
- Partials        4       13       +9

Files	Coverage Δ
...ks-popover/src/components/popover-content-core.tsx	`100.00% <100.00%> (ø)`

... and 46 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 dcafa86...c4db32e. Read the comment docs.

github-actions · 2024-06-27T21:09:57Z

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (f5c3293) 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="PR2263"

Packages can also be installed manually by running:

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

catandthemachines · 2024-06-28T01:07:14Z

Could you add a unit test that ensures the ariaLabel is set when passed in?

catandthemachines · 2024-06-28T01:06:32Z

packages/wonder-blocks-popover/src/components/popover-content-core.tsx

@@ -70,6 +70,7 @@ export default class PopoverContentCore extends React.Component<Props> {

    render(): React.ReactNode {
        const {
+            "aria-label": ariaLabel,


Oh interesting, so the ariaLabel prop is exposed, but the value is not piped through to View component??

Yup it accepts all the aria-* labels by default, but we have to expose them individually @catandthemachines

jeresig

Ahh - sounds good, thanks!

packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx

jeresig

Oh yes, good idea, thank you

packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx

…r.test.tsx Co-authored-by: John Resig <[email protected]>

add aria label to view component

4a33e31

anakaren-rojas self-assigned this Jun 27, 2024

khan-actions-bot requested a review from a team June 27, 2024 21:04

anakaren-rojas requested review from handeyeco and catandthemachines and removed request for a team June 27, 2024 21:04

khan-actions-bot requested a review from a team June 27, 2024 21:06

adding changeset

d030072

catandthemachines approved these changes Jun 28, 2024

View reviewed changes

jeresig changed the title ~~add aria label to view component~~ Add aria label to Popover Content Jun 28, 2024

jeresig approved these changes Jun 28, 2024

View reviewed changes

added test for popover content core aria label check

9b484d5

anakaren-rojas commented Jun 28, 2024

View reviewed changes

packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx Outdated Show resolved Hide resolved

jeresig approved these changes Jun 28, 2024

View reviewed changes

packages/wonder-blocks-popover/src/components/__tests__/popover.test.tsx Outdated Show resolved Hide resolved

anakaren-rojas and others added 2 commits June 28, 2024 12:49

Update packages/wonder-blocks-popover/src/components/__tests__/popove…

952f3d2

…r.test.tsx Co-authored-by: John Resig <[email protected]>

update test to disable lint check

c4db32e

anakaren-rojas changed the title ~~Add aria label to Popover Content~~ fix(LEMS-2086): Add aria label to Popover Content Jul 1, 2024

anakaren-rojas merged commit 78ee53c into main Jul 1, 2024
24 checks passed

anakaren-rojas deleted the LEMS-2089-expose-aria-label branch July 1, 2024 16:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(LEMS-2086): Add aria label to Popover Content #2263

fix(LEMS-2086): Add aria label to Popover Content #2263

anakaren-rojas commented Jun 27, 2024 •

edited

Loading

changeset-bot bot commented Jun 27, 2024 •

edited

Loading

khan-actions-bot commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

codecov bot commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

catandthemachines commented Jun 28, 2024

catandthemachines Jun 28, 2024

anakaren-rojas Jun 28, 2024

jeresig left a comment

jeresig left a comment

fix(LEMS-2086): Add aria label to Popover Content #2263

fix(LEMS-2086): Add aria label to Popover Content #2263

Conversation

anakaren-rojas commented Jun 27, 2024 • edited Loading

Summary:

Test plan:

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

🦋 Changeset detected

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

Gerald

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

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

A new build was pushed to Chromatic! 🚀

Chromatic results:

codecov bot commented Jun 27, 2024 • edited Loading

Codecov Report

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

npm Snapshot: Published

catandthemachines commented Jun 28, 2024

catandthemachines Jun 28, 2024

Choose a reason for hiding this comment

anakaren-rojas Jun 28, 2024

Choose a reason for hiding this comment

jeresig left a comment

Choose a reason for hiding this comment

jeresig left a comment

Choose a reason for hiding this comment

anakaren-rojas commented Jun 27, 2024 •

edited

Loading

changeset-bot bot commented Jun 27, 2024 •

edited

Loading

khan-actions-bot commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

codecov bot commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading