fix(Checkbox|Input): fix handling of aria-attributes #1752
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
layershifter
commented
Jun 9, 2017
src/lib/htmlInputPropsUtils.js
Outdated
| 'aria-readonly', 'aria-relevant', 'aria-required', 'aria-selected', 'aria-setsize', 'aria-sort', 'aria-valuemax', | ||
| 'aria-valuemin', 'aria-valuenow', 'aria-valuetext', 'role', | ||
| ] | ||
|
|
There was a problem hiding this comment.
@levithomason I'm not sure that we should pass all aria attributes to input in these elements, possible you'll want to perform a cleanup in the list.
There was a problem hiding this comment.
How about we update the existing partitionHTMLInputProps loop to check for aria- prefixed props instead? If /^aria-.*$/.test(prop) then it is automatically an inputProp and not a rest prop.
Codecov Report
@@ Coverage Diff @@
## master #1752 +/- ##
==========================================
+ Coverage 99.75% 99.75% +<.01%
==========================================
Files 142 142
Lines 2466 2467 +1
==========================================
+ Hits 2460 2461 +1
Misses 6 6
Continue to review full report at Codecov.
|
layershifter
commented
Jun 12, 2017
There was a problem hiding this comment.
@levithomason I've performed there changes changes that you've requested, looks cleaner 👍
| const { | ||
| htmlProps = htmlInputProps, | ||
| includeAria = true, | ||
| } = options |
There was a problem hiding this comment.
I've applied there our options pattern, so we can control htmlProps and ariaProps separately.
| const inputProps = {} | ||
| const rest = {} | ||
|
|
||
| _.forEach(props, (val, prop) => { | ||
| const target = _.includes(htmlProps, prop) ? inputProps : rest | ||
| const possibleAria = includeAria && (/^aria-.*$/.test(prop) || prop === 'role') |
There was a problem hiding this comment.
role prop belongs to aria attributes, too.
|
Looks great 👍 |
aabustamante
added a commit
to aabustamante/Semantic-UI-React
that referenced
this pull request
Jun 15, 2017
* docs(Button): remove redundant prop in Vertical Group example (Semantic-Org#1699) * feat(typescript): Export generics types (Semantic-Org#1698) * docs(Dropdown): fix world icon in search example (Semantic-Org#1695) * Fix MenuExampleText * add world icon to Dropdown example * rename 'languageOptions' to 'languages' * rename languages to languageOptions and separate line into two * separate line of code in two * separate props in different lines * remove trailing spaces * docs(Introduction): fix declarative example (Semantic-Org#1704) * feat(Item): add unstackable prop to ItemGroup (Semantic-Org#1706) * feat(Item): add unstackable prop to ItemGroup * docs(Item): add example for unstackable * chore(package): commit package-lock.json * 0.68.4 * docs(changelog): update changelog [ci skip] * docs(Icon): fix selector for input (Semantic-Org#1714) * chore(package): update require-dir to version 0.3.2 (Semantic-Org#1721) https://greenkeeper.io/ * docs(ItemExampleFloated): your description (Semantic-Org#1719) Fixes error in docs. "AS" is not being recognized. * chore(package): update react-ace to version 5.0.1 (Semantic-Org#1712) https://greenkeeper.io/ * fix(Dropdown): add addition item key (Semantic-Org#1727) * fix(factories): handle falsy `key` values (Semantic-Org#1729) * fix(Dropdown): fix key handling * fix(Dropdown): fix key handling * fix(factories): handle falsy keys * chore(package): update package-lock.json * chore(package): update chai-enzyme to 0.7.1 (Semantic-Org#1731) * feat(typings): expose FormComponent in typings (Semantic-Org#1680) * Exposed form component in typings to support custom controls in packages. * Update index.d.ts * Update index.d.ts * chore(package): update package-lock.json * 0.68.5 * docs(changelog): update changelog [ci skip] * fix(Input): add missing minLength prop (Semantic-Org#1734) * docs(TableExampleSortable): pass in null when that column shouldn't be sorted (Semantic-Org#1737) * feat(TextArea): add minHeight property, docs example (Semantic-Org#1679) * add minHeight property, example to docs * add rows prop/adjust minHeight prop usage * mixed(TextArea): update docs, tests, props and typings * fix(Textarea): move back minHeight prop to style * fix(htmlInputProps): fix handle on falsy values (Semantic-Org#1746) * fix(Search): Allow default action if there is no selected result (Semantic-Org#1742) * docs(images): add missing images, update urls (Semantic-Org#1763) * fix(Accordion): typings inverted to boolean (Semantic-Org#1758) Tiny fix for typings, inverted type incorrectly set to string. * feat(Icon): add ability use the loading prop without an icon (Semantic-Org#1768) * feat(Button): add focus method (Semantic-Org#1764) * fix(Dropdown): change active item on keyboard up/down (Semantic-Org#1735) * fix(Dropdown): change active item on keyboard up/down * fix(Dropdown): change active item on keyboard up/down * refactor(Dropdown): simplify move constant * refactor(Dropdown): remove hidden select (Semantic-Org#1730) breaking(Dropdown): remove hidden select * fix(Checkbox|Input): fix handling of aria-attributes (Semantic-Org#1752) * fix(Checkbox|Input): fix handling of aria-attributes * feat(htmlInputProps): update handling of aria
|
Released in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1750.