fix(UIshell): adds story for SkipToContent and removes the tabIndex prop requirement #4875
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
ghost
requested review from
abbeyhrt
changed the title
|
Deploy preview for the-carbon-components ready! Built with commit bbc60a7 https://deploy-preview-4875--the-carbon-components.netlify.com |
|
Deploy preview for carbon-components-react failed. Built with commit bbc60a7 https://app.netlify.com/sites/carbon-components-react/deploys/5df803eb499b9d0008a84fc0 |
|
Deploy preview for carbon-elements ready! Built with commit bbc60a7 |
aledavila
reviewed
Dec 13, 2019
|
If it was going from not required to required, it definitely would be but since it shouldn’t affect anybody using tabIndex prop, I think we’re good? @joshblack, what do you think? |
joshblack
approved these changes
Dec 16, 2019
|
It should be good! I think it falls under: |
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.
Closes #4802
This PR adds a story that more explicitly shows the use of
SkipToContent. Because we use render props in all of the current stories that useSkipToContent, the Story Source doesn't provide prop tables or component names for them.Example from Header Base w/ Navigation, Actions, and SideNav:
The new story shows all of the components used and their prop tables and is the only story that shows the code example for SkipToContent.
I'm waiting on confirmation from Sharon Sniders, but I believe the DAP violation related to SKipToContent is coming from when it is not visible and is not present when it is visible so it might not need to be addressed. If it does, #4865 will resolve the DAP violation.
In the comment for the
tabIndexproptype check, it is described as optional so I removed the.isRequiredfrom the check and changed the comment for thechildrenprop since it seemed to be describing the wrong thing.Changelog
New
SkipToContentuseChanged
StoryContentso there is something to tab to whenSkipToContentis used.Removed
.isRequiredfromtabIndexproptype checkTesting / Reviewing
Look at the story source for the new UIShell Story "Header Base w/ SkipToContent" and decide if we want this story included!