refactor: Consolidate showDropdown props #253

mrchief · 2019-05-18T01:00:14Z

BREAKING CHANGE: showDropdown & showDropdownAlways props are merged

showDropdown and showDropdownAlways props are now moved under showDropdown prop.

// before
<DropdownTreeSelect data={data} showDropdown />

// after
<DropdownTreeSelect data={data} showDropdown="initial" />

showDropdown: always

// before
<DropdownTreeSelect data={data} showDropdownAlways />

// after
<DropdownTreeSelect data={data} showDropdown="always" />

BREAKING: Property changes | Description | Usage before | Usage after | | --------------------------------------- | ----------------------------------------------------------- | ------------------------------------------------------------- | | Added a new `mode` prop | `simpleSelect={true}` / `simpleSelect` | `mode='simpleSelect'` | | Bundled text props into a single object | `placeholderText='My text'`<br>`noMatchesText='No matches'` | `texts={{ placeholder: 'My text', noMatches: 'No matches' }}` |

BREAKING: `hierarchical` prop `hierarchical` prop is now moved under `mode` prop. ``` // before <DropdownTreeSelect data={data} hierarchical={true} /> // after <DropdownTreeSelect data={data} mode="hierarchical" /> ```

src/a11y/a11y.test.js

coveralls · 2019-05-20T22:20:15Z

Pull Request Test Coverage Report for Build 1205

9 of 9 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.4%) to 94.668%

Totals
Change from base Build 1203:	0.4%
Covered Lines:	565
Relevant Lines:	580

💛 - Coveralls

ellinge · 2019-05-20T22:33:54Z

src/tree/index.js

+    }
+
+    if (/multiSelect|hierarchical/.test(mode)) {
+      attributes['aria-multiselectable'] = true


The reason for 'true' : 'false' before is because that forces the values to be outputted, not only on true (boolean true).

Not sure if we should keep that (does not seem to cause any a11y-errors when running tests)

return { /* https://www.w3.org/TR/wai-aria-1.1/#select * https://www.w3.org/TR/wai-aria-1.1/#tree */ role: mode === 'simpleSelect' ? 'listbox' : 'tree', 'aria-multiselectable': /multiSelect|hierarchical/.test(mode) ? 'true' : 'false', }

I guess this needs revisiting. It was complaining about them earlier but that could have been due to something else.

If all passes now, then we can keep the true/false

We need them to be strings for them to be emitted on false also.

I think the reason a11y tests pass is because of this:

from https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/listbox_role

aria-multiselectable
Include and set to true if the user can select more than one option. If set to true, every option should have a the aria-selected attribute included. If false or omitted, only the currently selected option, if any option is selected, needs the aria-selected attribute, and it must be set to true.

So going by this, it's ok to omit for single select cases.

# Conflicts: # README.md # __snapshots__/src/index.test.js.md # __snapshots__/src/index.test.js.snap # docs/src/stories/Options/index.js # src/a11y/a11y.test.js # src/index.js # src/index.test.js # src/tree-manager/tests/radioSelect.test.js # src/tree/index.js # types/react-dropdown-tree-select.d.ts

stale · 2019-06-05T17:41:53Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

codeclimate · 2019-06-05T19:03:50Z

Code Climate has analyzed commit b18136d and detected 0 issues on this pull request.

View more on Code Climate.

* feat: Group logically related props together (#242) ⚡️ BREAKING CHANGE: Property changes | Description | Usage before | Usage after | | --------------------------------------- | ----------------------------------------------------------- | ------------------------------------------------------------- | | Added a new `mode` prop | `simpleSelect={true}` / `simpleSelect` | `mode='simpleSelect'` | | Bundled text props into a single object | `placeholderText='My text'`<br>`noMatchesText='No matches'` | `texts={{ placeholder: 'My text', noMatches: 'No matches' }}` |

BREAKING CHANGE: Property changes | Description | Usage before | Usage after | | --------------------------------------- | ----------------------------------------------------------- | ------------------------------------------------------------- | | Added a new `mode` prop | `simpleSelect={true}` / `simpleSelect` | `mode='simpleSelect'` | | Bundled text props into a single object | `placeholderText='My text'`<br>`noMatchesText='No matches'` | `texts={{ placeholder: 'My text', noMatches: 'No matches' }}` |

Keeps dropdown open if updating with new props. Only updates/sets state open if new props cause the dropdown to show (a.k.a `initial`/`always`) #253 introduced resetting of state for shopDropdown on new props:

ellinge and others added 4 commits May 9, 2019 16:12

refactor: Update mode to include hierarchical (#251)

8d064fb

BREAKING: `hierarchical` prop `hierarchical` prop is now moved under `mode` prop. ``` // before <DropdownTreeSelect data={data} hierarchical={true} /> // after <DropdownTreeSelect data={data} mode="hierarchical" /> ```

docs: Consolidate showDropdown props

2f749cb

feat: Group showDropdown prop

9089e12

mrchief commented May 18, 2019

View reviewed changes

src/a11y/a11y.test.js Show resolved Hide resolved

mrchief and others added 5 commits May 17, 2019 21:04

chore: Remove console log

5e19d5c

fix: Fix aria attributes

83ae5ed

fix: Aria attributes

d72c255

fix: Aria attributes

0b1806a

fix: A11y error for radio

b51121f

ellinge previously approved these changes May 20, 2019

View reviewed changes

ellinge reviewed May 20, 2019

View reviewed changes

test: Test a11y for hierarchical mode as well

bf7bfd6

mrchief force-pushed the develop branch from 8d064fb to 8a9d148 Compare May 21, 2019 01:54

refactor: Always emit aria-multiselectable attribute

269634a

mrchief dismissed ellinge’s stale review via 269634a May 21, 2019 02:00

mrchief and others added 2 commits May 20, 2019 22:01

Merge branch 'develop' into feat/group_showDropdown

2f6a695

stale bot added the stale label Jun 5, 2019

Merge branch 'develop' into feat/group_showDropdown

b18136d

stale bot removed the stale label Jun 5, 2019

mrchief merged commit 297ed42 into develop Jun 10, 2019

mrchief deleted the feat/group_showDropdown branch June 10, 2019 02:08

ellinge mentioned this pull request Jun 14, 2019

fix: Remove resetting of dropdown on init props (#265) 🐛 #265

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Consolidate showDropdown props #253

refactor: Consolidate showDropdown props #253

mrchief commented May 18, 2019 •

edited

Loading

coveralls commented May 20, 2019 •

edited

Loading

ellinge May 20, 2019

mrchief May 20, 2019 •

edited

Loading

ellinge May 21, 2019

mrchief Jun 10, 2019

stale bot commented Jun 5, 2019

codeclimate bot commented Jun 5, 2019

refactor: Consolidate showDropdown props #253

refactor: Consolidate showDropdown props #253

Conversation

mrchief commented May 18, 2019 • edited Loading

coveralls commented May 20, 2019 • edited Loading

Pull Request Test Coverage Report for Build 1205

💛 - Coveralls

ellinge May 20, 2019

Choose a reason for hiding this comment

mrchief May 20, 2019 • edited Loading

Choose a reason for hiding this comment

ellinge May 21, 2019

Choose a reason for hiding this comment

mrchief Jun 10, 2019

Choose a reason for hiding this comment

stale bot commented Jun 5, 2019

codeclimate bot commented Jun 5, 2019

mrchief commented May 18, 2019 •

edited

Loading

coveralls commented May 20, 2019 •

edited

Loading

mrchief May 20, 2019 •

edited

Loading