feat(mixed): add accessibility prop to all components
#927
Conversation
layershifter
requested review from
dvdzkwsk,
kuzhelov,
levithomason,
miroslavstastny and
mnajdova
as code owners
Codecov Report
@@ Coverage Diff @@
## master #927 +/- ##
==========================================
+ Coverage 80.43% 80.51% +0.08%
==========================================
Files 659 659
Lines 8412 8447 +35
Branches 1424 1492 +68
==========================================
+ Hits 6766 6801 +35
Misses 1631 1631
Partials 15 15
Continue to review full report at Codecov.
|
| * Accessibility behavior if overridden by the user. | ||
| * @default defaultBehavior | ||
| */ | ||
| accessibility?: Accessibility |
There was a problem hiding this comment.
Can we add this in interface (or if it should be use in all UI components, maybe even move to the UIComponentProps)? At least common interface would allow us to reuse the same description, although not sure how we can add different @defaults there..
There was a problem hiding this comment.
I'm note sure that the issues with @default will be solved right now while some components have different defaults 🐱
| @@ -46,19 +54,22 @@ class ChatItem extends UIComponent<ReactProps<ChatItemProps>, any> { | |||
|
|
|||
| static propTypes = { | |||
| ...commonPropTypes.createCommon({ content: false }), | |||
| accessibility: customPropTypes.accessibility, | |||
There was a problem hiding this comment.
I would definitely expect this to be in the commonPropTypes.createCommon factory.. We can use {accessibility: false} if we don't want it there for some components
There was a problem hiding this comment.
I like createCommon(), but #647 (comment)
There was a problem hiding this comment.
Ok, if we agree to remove this abstraction, then we should do this, but having now some property being defined directly, and some of them as result of a function is really confusing. Let's create then issue for this, because I missed that comment and was not aware of it by now...
layershifter
force-pushed
the
fix/accept-a11y
branch
from
fc605f5 to
ecd337c
Compare
…tardust-ui/react into fix/accept-a11y # Conflicts: # packages/react/src/components/Status/Status.tsx
| @@ -104,7 +104,7 @@ class Accordion extends AutoControlledComponent<ReactProps<AccordionProps>, any> | |||
| }), | |||
| ), | |||
| ]), | |||
| accessibility: PropTypes.func, | |||
| accessibility: customPropTypes.accessibility, | |||
There was a problem hiding this comment.
we can remove this from here now
| @@ -197,6 +197,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |||
|
|
|||
| static propTypes = { | |||
| ...commonPropTypes.createCommon({ | |||
| accessibility: false, | |||
There was a problem hiding this comment.
Are we sure which component should not have the accessibility property?
There was a problem hiding this comment.
Yep, it's not implemented yet #875
Codecov Report
@@ Coverage Diff @@
## master #927 +/- ##
==========================================
+ Coverage 80.43% 80.51% +0.08%
==========================================
Files 659 659
Lines 8412 8447 +35
Branches 1424 1492 +68
==========================================
+ Hits 6766 6801 +35
Misses 1631 1631
Partials 15 15
Continue to review full report at Codecov.
|
Fixes #633.