refactor(Banner): update region to use a dedicated aria-label #4539

joshblack · 2024-04-26T22:03:08Z

Closes https://github.com/github/primer/issues/3262

Changelog

New

Changed

Update Banner based on the changes to the API doc so that it is no longer aria-labelledby the heading and is instead aria-label with the variant info

Removed

Rollout strategy

Minor release

Testing & Reviewing

Verify that the change reflects the API docs changes
Make sure that test scenarios have been accurately updated to reflect the changes
Is there anything I'm missing in this implementation? 👀

github-actions · 2024-04-26T22:06:42Z

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	88.04 KB (0%)
packages/react/dist/browser.umd.js	88.38 KB (0%)

packages/react/src/Banner/Banner.test.tsx

changeset-bot · 2024-04-29T20:42:10Z

🦋 Changeset detected

Latest commit: 1366cdc

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

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

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

khiga8

🎉

packages/react/src/Banner/Banner.test.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

broccolinisoup

Just left one question 😊 Looks good!

broccolinisoup · 2024-04-30T23:52:30Z

packages/react/src/Banner/Banner.tsx

        throw new Error(
-          'The Banner component requires a title to be provided as the `title` prop or through `Banner.Title`',
+          'Expected a title to be provided to the <Banner> component with the `title` prop or through `<Banner.Title>` but no title was found',


What is a good way to differentiate Error vs console.error (or invariant) in these type of scenarios like when to use one over the other one? 👀

@broccolinisoup great question honestly, they kind of each have a niche and I'm not sure when it's best to use which 🤔 Would this be a good topic for a working session?

I think it is a great idea 🔥 I had related questions in mind as well. I'll add it to the agenda!

* refactor(Banner): update region to use a dedicated aria-label * chore: add changeset, update config to exclude codesandbox * feat: add aria-label to Banner * Update packages/react/src/Banner/Banner.test.tsx Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> * Update packages/react/src/Banner/Banner.test.tsx Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> * test: update test label with aria-label --------- Co-authored-by: Josh Black <joshblack@users.noreply.github.com> Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* Add new props to `FormControl.Label` * Add conditional * Add changeset * Update docs json * Add more examples * chore(deps-dev): bump ejs from 3.1.9 to 3.1.10 (#4549) Bumps [ejs](https://github.com/mde/ejs) from 3.1.9 to 3.1.10. - [Release notes](https://github.com/mde/ejs/releases) - [Commits](mde/ejs@v3.1.9...v3.1.10) --- updated-dependencies: - dependency-name: ejs dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * BranchName: Add style for span and add v8 tokens (#4556) * add style for branchName as span adn add v8 tokens * added changeset * Update thin-ligers-turn.md * test(vrt): update snapshots * use not a instead of matching for span --------- Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com> Co-authored-by: lukasoppermann <lukasoppermann@users.noreply.github.com> * refactor(Banner): update region to use a dedicated aria-label (#4539) * refactor(Banner): update region to use a dedicated aria-label * chore: add changeset, update config to exclude codesandbox * feat: add aria-label to Banner * Update packages/react/src/Banner/Banner.test.tsx Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> * Update packages/react/src/Banner/Banner.test.tsx Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> * test: update test label with aria-label --------- Co-authored-by: Josh Black <joshblack@users.noreply.github.com> Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> * chore(deps-dev): bump cross-env from 7.0.2 to 7.0.3 (#4561) Bumps [cross-env](https://github.com/kentcdodds/cross-env) from 7.0.2 to 7.0.3. - [Release notes](https://github.com/kentcdodds/cross-env/releases) - [Changelog](https://github.com/kentcdodds/cross-env/blob/master/CHANGELOG.md) - [Commits](kentcdodds/cross-env@v7.0.2...v7.0.3) --- updated-dependencies: - dependency-name: cross-env dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump @babel/plugin-transform-modules-commonjs (#4562) Bumps [@babel/plugin-transform-modules-commonjs](https://github.com/babel/babel/tree/HEAD/packages/babel-plugin-transform-modules-commonjs) from 7.23.3 to 7.24.1. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.24.1/packages/babel-plugin-transform-modules-commonjs) --- updated-dependencies: - dependency-name: "@babel/plugin-transform-modules-commonjs" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump jest-fail-on-console from 3.1.1 to 3.2.0 (#4563) Bumps [jest-fail-on-console](https://github.com/ValentinH/jest-fail-on-console) from 3.1.1 to 3.2.0. - [Release notes](https://github.com/ValentinH/jest-fail-on-console/releases) - [Commits](ValentinH/jest-fail-on-console@v3.1.1...v3.2.0) --- updated-dependencies: - dependency-name: jest-fail-on-console dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat(FeatureFlags): broaden feature flag type to accept undefined (#4554) * feat(FeatureFlags): loosen feature flag type to accept undefined * chore: add changeset * Update .changeset/grumpy-coats-worry.md --------- Co-authored-by: Josh Black <joshblack@users.noreply.github.com> * prevent form submit (#4551) * deprecate title prop on ActionList.Group component on docs (#4544) * chore: add hydro analytics to storybook (#4558) * chore: add hydro analytics to storybook * chore: use previewHead over managerHead * Update build-docs --------- Co-authored-by: Josh Black <joshblack@users.noreply.github.com> * Revert "Revert "Add support for nested submenus to `ActionMenu`"" (#4486) * Revert "Revert "Add support for nested submenus to `ActionMenu` (#4386)" (#4472)" This reverts commit 82072eb. * just want a change to trigger rebuild --------- Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com> Co-authored-by: Pavithra Kodmad <pksjce@github.com> * chore(deps): update typescript to 5.4.5 (#4568) Co-authored-by: Josh Black <joshblack@users.noreply.github.com> * Use dynamic height and width for dialogs (#4567) * Use dynamic height and width for dialogs * Update tall-forks-bathe.md --------- Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com> * Make asterisk default, update story scenarios * Update packages/react/src/FormControl/FormControl.docs.json Co-authored-by: Owen Niblock <owenniblock@github.com> * Update packages/react/src/internal/components/InputLabel.tsx Co-authored-by: Owen Niblock <owenniblock@github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lukas Oppermann <lukasoppermann@github.com> Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com> Co-authored-by: lukasoppermann <lukasoppermann@users.noreply.github.com> Co-authored-by: Josh Black <joshblack@github.com> Co-authored-by: Josh Black <joshblack@users.noreply.github.com> Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> Co-authored-by: Armağan <broccolinisoup@github.com> Co-authored-by: Ian Sanders <iansan5653@github.com> Co-authored-by: Pavithra Kodmad <pksjce@github.com> Co-authored-by: Dusty Greif <dgreif@users.noreply.github.com> Co-authored-by: Owen Niblock <owenniblock@github.com>

joshblack added 2 commits April 26, 2024 17:01

refactor(Banner): update region to use a dedicated aria-label

9568dc6

chore: add changeset, update config to exclude codesandbox

6e1c409

joshblack changed the title ~~Feat/remove title id requirement~~ refactor(Banner): update region to use a dedicated aria-label Apr 26, 2024

joshblack temporarily deployed to github-pages April 26, 2024 22:06 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4539 April 26, 2024 22:06 Inactive

joshblack marked this pull request as ready for review April 29, 2024 14:48

joshblack requested a review from a team as a code owner April 29, 2024 14:48

joshblack requested review from langermank and khiga8 April 29, 2024 14:48

khiga8 reviewed Apr 29, 2024

View reviewed changes

packages/react/src/Banner/Banner.test.tsx Show resolved Hide resolved

feat: add aria-label to Banner

e89bd6a

joshblack requested a review from khiga8 April 29, 2024 20:42

joshblack temporarily deployed to github-pages April 29, 2024 20:45 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4539 April 29, 2024 20:46 Inactive

khiga8 approved these changes Apr 30, 2024

View reviewed changes

packages/react/src/Banner/Banner.test.tsx Outdated Show resolved Hide resolved

packages/react/src/Banner/Banner.test.tsx Outdated Show resolved Hide resolved

joshblack and others added 2 commits April 30, 2024 11:12

Update packages/react/src/Banner/Banner.test.tsx

ff471fa

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

Update packages/react/src/Banner/Banner.test.tsx

7e13350

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

joshblack temporarily deployed to github-pages April 30, 2024 16:16 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4539 April 30, 2024 16:16 Inactive

joshblack requested a review from broccolinisoup April 30, 2024 22:09

broccolinisoup approved these changes Apr 30, 2024

View reviewed changes

Merge branch 'main' into feat/remove-title-id-requirement

c008782

joshblack temporarily deployed to github-pages May 1, 2024 19:08 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4539 May 1, 2024 19:09 Inactive

joshblack and others added 2 commits May 3, 2024 15:59

test: update test label with aria-label

663af56

Merge branch 'main' into feat/remove-title-id-requirement

1366cdc

joshblack enabled auto-merge May 3, 2024 20:59

joshblack deployed to github-pages May 3, 2024 21:02 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-4539 May 3, 2024 21:02 View deployment

joshblack added this pull request to the merge queue May 3, 2024

Merged via the queue into main with commit f0de234 May 3, 2024
30 checks passed

joshblack deleted the feat/remove-title-id-requirement branch May 3, 2024 21:18

primer bot mentioned this pull request May 3, 2024

Release tracking #4559

Merged

This was referenced Jun 11, 2024

[Snyk] Upgrade @primer/react from 36.18.0 to 36.19.0 JonathanXDR/Atom#27

Closed

[Snyk] Upgrade @primer/react from 36.18.0 to 36.19.1 JonathanXDR/Atom#29

Closed

This was referenced Jun 20, 2024

[Snyk] Upgrade @primer/react from 36.18.0 to 36.19.1 JonathanXDR/Atom#30

Closed

[Snyk] Upgrade @primer/react from 36.18.0 to 36.20.0 JonathanXDR/Atom#31

Closed

StemmlerSisters mentioned this pull request Jul 16, 2024

[Snyk] Upgrade @primer/react from 36.1.0 to 36.23.0 StemmlerSisters/documentation#5

Open

snyk-io bot mentioned this pull request Jul 17, 2024

[Snyk] Upgrade @primer/react from 36.11.0 to 36.23.0 Hawthorne001/documentation.npm#3

Open

julitabarelkowska mentioned this pull request Jul 17, 2024

[Snyk] Upgrade @primer/react from 36.11.0 to 36.23.0 julitabarelkowska/documentation#7

Open

Canadatechmanufacturing mentioned this pull request Jul 17, 2024

[Snyk] Upgrade @primer/react from 36.7.0 to 36.23.0 Canadatechmanufacturing/documentation#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(Banner): update region to use a dedicated aria-label #4539

refactor(Banner): update region to use a dedicated aria-label #4539

joshblack commented Apr 26, 2024 •

edited

Loading

github-actions bot commented Apr 26, 2024 •

edited

Loading

changeset-bot bot commented Apr 29, 2024 •

edited

Loading

khiga8 left a comment

broccolinisoup left a comment

broccolinisoup Apr 30, 2024

joshblack May 1, 2024

broccolinisoup May 6, 2024

refactor(Banner): update region to use a dedicated aria-label #4539

refactor(Banner): update region to use a dedicated aria-label #4539

Conversation

joshblack commented Apr 26, 2024 • edited Loading

Changelog

New

Changed

Removed

Rollout strategy

Testing & Reviewing

github-actions bot commented Apr 26, 2024 • edited Loading

size-limit report 📦

changeset-bot bot commented Apr 29, 2024 • edited Loading

🦋 Changeset detected

khiga8 left a comment

Choose a reason for hiding this comment

broccolinisoup left a comment

Choose a reason for hiding this comment

broccolinisoup Apr 30, 2024

Choose a reason for hiding this comment

joshblack May 1, 2024

Choose a reason for hiding this comment

broccolinisoup May 6, 2024

Choose a reason for hiding this comment

joshblack commented Apr 26, 2024 •

edited

Loading

github-actions bot commented Apr 26, 2024 •

edited

Loading

changeset-bot bot commented Apr 29, 2024 •

edited

Loading