feat(radio): Radio group compliant with ARIA patterns #229

jurokapsiar · 2018-09-14T05:45:52Z

RadioGroup

This is a ARIA-patterns compliant implementation of Radio and RadioGroup.
My initial testing shows that it works with all required screen reader/browser combinations.
The only exception is that the focused option is not highlighted with JAWS in virtual cursor mode.

This breaks the current Radio API as Radio is replaced with RadioGroup.Item

TODO

API Proposal

[Group]

<RadioGroup defaultCheckedIndex={0} items={items} onChange={this.handleChange} />

  <div role="radiogroup" class="ui-radiogroup">
    <div role="radio" tabindex="0" aria-checked="true" index="0" class="ui-radio f g">
      <span class="ui-label ...">
        <span class="ui-icon ..." aria-hidden="true"></span>
        Capricciosa
      </span>
    </div>
  </div>

Comparison of approaches that I tried: (the last row is this implementation)

… the Radio component

…eact into feat/radio-checked

-reverted the Radio to extend UIComponent

-styles the Radio button with custom icon

… icon's margin

-addressed comments in the PR

This reverts commit ec60c8b.

# Conflicts: # docs/src/examples/components/Radio/index.tsx # src/components/Radio/Radio.tsx # src/index.ts # src/themes/teams/components/Radio/radioStyles.ts # src/themes/teams/components/Radio/radioVariables.ts

mnajdova · 2018-09-14T09:08:41Z

CHANGELOG.md

@@ -34,6 +34,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Fixed missing colors in Teams' siteVariables @mnajdova ([#200](https://github.com/stardust-ui/react/pull/200))
 - Fixed Teams' siteVariables font sizes @levithomason ([#204](https://github.com/stardust-ui/react/pull/204))
 - Fixed docs examples of `Text` component @codepretty ([#205](https://github.com/stardust-ui/react/pull/205))
+- Add `value`, `disabled`, `checked`, `defaultChecked` and `onChange` props to `Radio` component @mnajdova ([#206](https://github.com/stardust-ui/react/pull/206))
 - Preserve fonts and static styles in mergeThemes @levithomason ([#217](https://github.com/stardust-ui/react/pull/217))


Please delete this from here, it was added in the unreleased section

mnajdova · 2018-09-14T09:27:29Z

src/components/Radio/RadioGroup.tsx

+        defaultProps: {
+          index,
+          checked: parseInt(checkedIndex, 10) === index,
+        },


Maybe is better that we use the value for this instead of index? Is there reason why we are using index?

src/lib/accessibility/Behaviors/Radio/RadioBehavior.ts

src/lib/accessibility/Behaviors/Radio/RadioGroupBehavior.ts

src/lib/accessibility/Behaviors/Radio/RadioBehavior.ts

codecov · 2018-09-19T04:44:16Z

Codecov Report

Merging #229 into master will decrease coverage by 2.01%.
The diff coverage is 71.96%.

@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
- Coverage   91.84%   89.82%   -2.02%     
==========================================
  Files          61       62       +1     
  Lines        1042     1111      +69     
  Branches      134      166      +32     
==========================================
+ Hits          957      998      +41     
- Misses         82      110      +28     
  Partials        3        3

Impacted Files	Coverage Δ
src/index.ts	`100% <100%> (ø)`	⬆️
src/components/RadioGroup/index.ts	`100% <100%> (ø)`
src/components/Button/Button.tsx	`95.83% <100%> (-0.09%)`	⬇️
src/components/RadioGroup/RadioGroup.tsx	`51.72% <51.72%> (ø)`
src/components/RadioGroup/RadioGroupItem.tsx	`94.73% <94.73%> (ø)`
src/components/Provider/Provider.tsx	`60.46% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

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

codecov · 2018-09-19T04:47:08Z

Codecov Report

Merging #229 into master will decrease coverage by 2.01%.
The diff coverage is 71.96%.

@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
- Coverage   91.84%   89.82%   -2.02%     
==========================================
  Files          61       62       +1     
  Lines        1042     1111      +69     
  Branches      134      166      +32     
==========================================
+ Hits          957      998      +41     
- Misses         82      110      +28     
  Partials        3        3

Impacted Files	Coverage Δ
src/index.ts	`100% <100%> (ø)`	⬆️
src/components/RadioGroup/index.ts	`100% <100%> (ø)`
src/components/Button/Button.tsx	`95.83% <100%> (-0.09%)`	⬇️
src/components/RadioGroup/RadioGroup.tsx	`51.72% <51.72%> (ø)`
src/components/RadioGroup/RadioGroupItem.tsx	`94.73% <94.73%> (ø)`
src/components/Provider/Provider.tsx	`60.46% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

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

codecov · 2018-09-19T04:47:15Z

Codecov Report

Merging #229 into master will increase coverage by 0.41%.
The diff coverage is 97.27%.

@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
+ Coverage   92.15%   92.57%   +0.41%     
==========================================
  Files          61       62       +1     
  Lines        1058     1131      +73     
  Branches      155      168      +13     
==========================================
+ Hits          975     1047      +72     
- Misses         80       81       +1     
  Partials        3        3

Impacted Files	Coverage Δ
src/index.ts	`100% <100%> (ø)`	⬆️
src/components/RadioGroup/index.ts	`100% <100%> (ø)`
src/components/RadioGroup/RadioGroupItem.tsx	`100% <100%> (ø)`
src/components/Button/Button.tsx	`95.83% <100%> (-0.09%)`	⬇️
src/components/RadioGroup/RadioGroup.tsx	`95% <95%> (ø)`

Continue to review full report at Codecov.

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

mnajdova · 2018-09-19T13:04:50Z

test/specs/commonTests/implementsShorthandProp.tsx

@@ -3,10 +3,12 @@ import { mount } from './isConformant'

 export type ShorthandTestOptions = {
  mapsValueToProp?: string
+  skipArrayOfStrings?: boolean
 }



Let's not change this interface, but create new one specific for the collection shorthand, as skipArrayOfStrings is not in any way used in the simple shorthand. The last option how the shorthand can be used is a function with signature: (Component, props, children) => React.Node. If we are not supporting this one, maybe we should add config for this one as well.

mnajdova · 2018-09-19T13:47:23Z

As per our discussion the next step would be introducing variables for the RadioGroup with slot for the items, but we are leaving this for dedicated PR. In my opinion this is good to be merged now.

# Conflicts: # src/components/Radio/Radio.tsx

…add changelog

jurokapsiar · 2018-09-26T14:02:16Z

fixes #195

manajdov and others added 21 commits September 7, 2018 16:16

-added value, disabled, checked, defaultChecked and onChange props to…

2d0ea16

… the Radio component

-updated changelog

804811c

Merge branch 'master' into feat/radio-checked

efebc5e

-added name property on the Radio component

fb7eeb4

Merge branch 'feat/radio-checked' of https://github.com/stardust-ui/r…

5067012

…eact into feat/radio-checked

-introduced RadioGroup component

5a0a25d

-reverted the Radio to extend UIComponent

-changed example in the radio group

484c642

-small fix

1a9bc23

-removed auto controlled props

ef68706

-removed RadioGroup

64275dc

-styles the Radio button with custom icon

-adjusting the content of the Radio button and added variable for the…

d3b029b

… icon's margin

-removed the radio group example

0eb85bd

-addressed comments in the PR

accessible radio group

79fcb36

Merge branch 'master' into feat/radio-group-acc

ede90b4

focus indicator

851d2b5

accessibility using input element

ec60c8b

Revert "accessibility using input element"

567eb6e

This reverts commit ec60c8b.

conformance to aria best practices

705b9e1

Merge branch 'master' into feat/radio-group-acc-div

4960d5e

# Conflicts: # docs/src/examples/components/Radio/index.tsx # src/components/Radio/Radio.tsx # src/index.ts # src/themes/teams/components/Radio/radioStyles.ts # src/themes/teams/components/Radio/radioVariables.ts

fix issues after merge

be7b3e0

added few comments

c0e8728

jurokapsiar added RFC 🚧 WIP labels Sep 14, 2018

jurokapsiar requested review from dvdzkwsk, kuzhelov, levithomason, miroslavstastny and mnajdova as code owners September 14, 2018 05:45

mnajdova reviewed Sep 14, 2018

View reviewed changes

sophieH29 reviewed Sep 18, 2018

View reviewed changes

src/lib/accessibility/Behaviors/Radio/RadioBehavior.ts Outdated Show resolved Hide resolved

sophieH29 reviewed Sep 18, 2018

View reviewed changes

src/lib/accessibility/Behaviors/Radio/RadioGroupBehavior.ts Show resolved Hide resolved

sophieH29 reviewed Sep 18, 2018

View reviewed changes

src/lib/accessibility/Behaviors/Radio/RadioBehavior.ts Outdated Show resolved Hide resolved

allow using children api for items

afeba8e

kuzhelov added needs author changes Author needs to implement changes before merge and removed 🚀 ready for review labels Sep 18, 2018

Juraj Kapsiar and others added 4 commits September 18, 2018 17:58

various fixes from PR review

6dfec6e

do not use array.findindex

0629580

remove double condition

8d492cf

unit tests for RadioGroup

e44405b

Juraj Kapsiar added 2 commits September 19, 2018 14:33

better UT coverage

4ce9a90

add color piicker example

28746d4

mnajdova reviewed Sep 19, 2018

View reviewed changes

separate interface for implemensCollectionShorthandProp

99db39b

jurokapsiar added 🚀 ready for review and removed needs author changes Author needs to implement changes before merge labels Sep 19, 2018

mnajdova approved these changes Sep 19, 2018

View reviewed changes

Juraj Kapsiar and others added 4 commits September 19, 2018 19:39

Merge branch 'master' into feat/radio-group-acc-div

bad93f9

# Conflicts: # src/components/Radio/Radio.tsx

apply styles change to new files, add accessibility compliance test, …

a142b38

…add changelog

fix lint issue

34306df

Merge branch 'master' into feat/radio-group-acc-div

fa268a5

jurokapsiar changed the title ~~BREAKING: Feat/radio group compliant with ARIA patterns~~ Feat(radio): radio group compliant with ARIA patterns Sep 20, 2018

jurokapsiar changed the title ~~Feat(radio): radio group compliant with ARIA patterns~~ feat(radio): Radio group compliant with ARIA patterns Sep 20, 2018

jurokapsiar merged commit 5e5fac6 into master Sep 20, 2018

jurokapsiar mentioned this pull request Sep 26, 2018

[RFC] Radio - create group options and add event handlers #195

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(radio): Radio group compliant with ARIA patterns #229

feat(radio): Radio group compliant with ARIA patterns #229

jurokapsiar commented Sep 14, 2018 •

edited

mnajdova Sep 14, 2018

mnajdova Sep 14, 2018

codecov bot commented Sep 19, 2018

codecov bot commented Sep 19, 2018

codecov bot commented Sep 19, 2018 •

edited

mnajdova Sep 19, 2018

mnajdova commented Sep 19, 2018

jurokapsiar commented Sep 26, 2018

feat(radio): Radio group compliant with ARIA patterns #229

feat(radio): Radio group compliant with ARIA patterns #229

Conversation

jurokapsiar commented Sep 14, 2018 • edited

RadioGroup

TODO

API Proposal

[Group]

mnajdova Sep 14, 2018

Choose a reason for hiding this comment

mnajdova Sep 14, 2018

Choose a reason for hiding this comment

codecov bot commented Sep 19, 2018

Codecov Report

codecov bot commented Sep 19, 2018

Codecov Report

codecov bot commented Sep 19, 2018 • edited

Codecov Report

mnajdova Sep 19, 2018

Choose a reason for hiding this comment

mnajdova commented Sep 19, 2018

jurokapsiar commented Sep 26, 2018

jurokapsiar commented Sep 14, 2018 •

edited

codecov bot commented Sep 19, 2018 •

edited