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

feat(Header): addressing accessibility concerns and replacing subheader with description #17

Merged
merged 13 commits into from
Jul 31, 2018

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Jul 27, 2018

Header

Addressing accessibility concerns and replacing the subheader prop with description. Previous discussions: stardust-ui/react-old#123

TODO

  • Conformance test
  • Minimal doc site example
  • Stardust base theme
  • Teams Light theme
  • Teams Dark theme
  • Teams Contrast theme
  • Confirm RTL usage
  • W3 accessibility check
  • Stardust accessibility check
  • Update glossary props table
  • Update the CHANGELOG.md

…igin

# Conflicts:
#	CHANGELOG.md
#	src/components/Header/Header.tsx
@mnajdova mnajdova changed the title feat (Header): addressing accessibility concerns and replacing subheader with description feat(Header): addressing accessibility concerns and replacing subheader with description Jul 27, 2018
@kuzhelov
Copy link
Contributor

one thing that we might think about in addition - from a client's perspective it would be convenient to support the following way of declaring Header:

...
<Header ... description='some description as plain string' />

where description is provided as a plain string. Currently the only way to provide description in this case is by using the explicit way of declaring the object:

<Header ... description={{ content: '...' }}

While I do understand that currently taken approach is consistent by the means of library design, it seems to be a too complex way for a client to provide description in the simplest situations. Probably, we could support first more declarartive way of providing Header as well - for those cases where description is only about the content provided

@mnajdova
Copy link
Contributor Author

Hi Roman, actually defining the description is possible with the proposed code, I guess just the examples are misleading that it is not supported. I will change the example for showing the description prop to be used as your proposal. Thanks for the catch!

@codecov
Copy link

codecov bot commented Jul 31, 2018

Codecov Report

Merging #17 into master will increase coverage by 0.76%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
+ Coverage   83.58%   84.34%   +0.76%     
==========================================
  Files          59       59              
  Lines         804      792      -12     
  Branches      163      160       -3     
==========================================
- Hits          672      668       -4     
+ Misses        128      120       -8     
  Partials        4        4
Impacted Files Coverage Δ
src/components/Header/headerDescriptionRules.ts 100% <ø> (ø)
src/components/Header/Header.tsx 93.75% <100%> (+0.41%) ⬆️
src/components/Header/HeaderDescription.tsx 100% <100%> (ø)
src/components/Header/headerRules.ts 100% <100%> (ø) ⬆️
src/styles/customCSS.ts 100% <0%> (ø) ⬆️
src/components/Icon/Icon.tsx 100% <0%> (ø) ⬆️
src/components/Icon/iconVariables.ts 100% <0%> (ø) ⬆️
src/components/Avatar/avatarRules.ts 83.33% <0%> (ø) ⬆️
src/components/Icon/iconRules.ts 90.9% <0%> (+20.32%) ⬆️

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 81ca038...7e267ca. Read the comment docs.

@mnajdova mnajdova merged commit 953674c into master Jul 31, 2018
@layershifter layershifter deleted the header/addresing-accessibility-concerns branch January 10, 2019 11:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants