Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat(status): Add status behavior #880

Merged
merged 8 commits into from
Feb 19, 2019
Merged

Conversation

kolaps33
Copy link
Collaborator

@kolaps33 kolaps33 commented Feb 11, 2019

We recognized if status is used in context then the status itself is not navigable with screen reader in the reader mode (like virtual cursor, scan mode, browse mode).
Se we need to put status as img with proper alt attribute. Then the reader navigate on the status element in reader mode.

@codecov
Copy link

codecov bot commented Feb 11, 2019

Codecov Report

Merging #880 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #880      +/-   ##
==========================================
+ Coverage   80.37%   80.38%   +0.01%     
==========================================
  Files         656      657       +1     
  Lines        8393     8398       +5     
  Branches     1488     1424      -64     
==========================================
+ Hits         6746     6751       +5     
  Misses       1632     1632              
  Partials       15       15
Impacted Files Coverage Δ
packages/react/src/components/Dialog/Dialog.tsx 31.25% <ø> (ø) ⬆️
packages/react/src/lib/accessibility/index.ts 100% <100%> (ø) ⬆️
...b/accessibility/Behaviors/Status/statusBehavior.ts 100% <100%> (ø)
packages/react/src/index.ts 100% <100%> (ø) ⬆️
packages/react/src/components/Status/Status.tsx 100% <100%> (ø) ⬆️

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 d237dbd...2b20e57. Read the comment docs.

* @specification
* Adds role='img'.
*/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be there an empty line? 🤔

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a changelog entry 👍

@@ -28,6 +36,8 @@ export interface StatusProps extends UIComponentProps {

/**
* A status graphically represents someone's or something's state.
* @accessibility
* Don't forget to put appropriate 'alt' attribute describing status. This is necessary for correct screen reader behavior.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If alt prop is so necessary, should be a component prop?

@kolaps33 kolaps33 merged commit b0dd20c into master Feb 19, 2019
@delete-merged-branch delete-merged-branch bot deleted the mituron/status-screen-reader-mode branch February 19, 2019 18:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants