Skip to content

Conversation

@fa7ad
Copy link
Contributor

@fa7ad fa7ad commented Jan 23, 2023

Description

This PR adds the ariaControls prop to RadioGroup component.
This should allow users to implement a progressive disclosure pattern AND make it accessible.

NOTE: The API documentation needs to reviewed and potentially changed; hold off on merging just yet. approved

Related links, issue #, if available: n/a

How has this been tested?

  • Updated existing Unit Tests
  • Created a dev page that uses the prop to implement this pattern.
Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@fa7ad fa7ad requested a review from a team as a code owner January 23, 2023 10:20
@fa7ad fa7ad requested review from just-boris and removed request for a team January 23, 2023 10:20
@fa7ad fa7ad force-pushed the feat/progressive-disclosure branch 2 times, most recently from c58a3e7 to 80516a9 Compare January 23, 2023 10:22
@fa7ad fa7ad marked this pull request as draft January 23, 2023 10:23
@fa7ad fa7ad force-pushed the feat/progressive-disclosure branch from 80516a9 to 6c9784e Compare January 23, 2023 10:26
@fa7ad fa7ad changed the title WIP: feat: Add ariaControls prop to RadioGroup feat: Add ariaControls prop to RadioGroup Jan 23, 2023
@codecov
Copy link

codecov bot commented Jan 23, 2023

Codecov Report

Base: 92.98% // Head: 92.98% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (4cc79e6) compared to base (23f9839).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #659   +/-   ##
=======================================
  Coverage   92.98%   92.98%           
=======================================
  Files         574      574           
  Lines       16782    16783    +1     
  Branches     4632     4632           
=======================================
+ Hits        15604    15605    +1     
  Misses       1101     1101           
  Partials       77       77           
Impacted Files Coverage Δ
src/radio-group/internal.tsx 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@fa7ad fa7ad force-pushed the feat/progressive-disclosure branch 2 times, most recently from cf78290 to e90d8dc Compare January 25, 2023 10:07
@fa7ad fa7ad marked this pull request as ready for review January 25, 2023 10:11
@fa7ad fa7ad force-pushed the feat/progressive-disclosure branch 2 times, most recently from a8ed001 to 612efe6 Compare January 25, 2023 13:08
just-boris
just-boris previously approved these changes Jan 25, 2023
@just-boris just-boris dismissed their stale review January 25, 2023 15:38

misclick. Please check the comments

@timogasda timogasda self-requested a review January 25, 2023 15:53
@fa7ad fa7ad force-pushed the feat/progressive-disclosure branch from 612efe6 to 518bcd9 Compare January 25, 2023 17:43
@fa7ad fa7ad requested a review from just-boris January 25, 2023 18:07
just-boris
just-boris previously approved these changes Jan 25, 2023
@fa7ad fa7ad force-pushed the feat/progressive-disclosure branch from 36e4e34 to 4cc79e6 Compare January 26, 2023 11:28
@fa7ad fa7ad merged commit 7660254 into cloudscape-design:main Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants