ContextualMenu - Added support for custom component lengths and fixed aria-posinset value#4004
Merged
jspurlin merged 7 commits intomicrosoft:masterfrom Feb 21, 2018
Merged
Conversation
chang47
changed the title
dzearing
changed the title
jspurlin
approved these changes
Feb 20, 2018
joschect
approved these changes
Feb 21, 2018
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Problem:
In the Contextual Menu if we have to rely on the custom onRender() of a passed in ContextualItem we would pass in the index of our element so that the user can use it for aria-posinset for accessibility reasons.
However, in the event that the contextual item is a list and has items in it. The contextual menu would have no way of knowing.
Also, posinset starts at index 1 and not 0. We currently pass in our index starting at the value of 0, so I'm also doing a side fix for that for the custom onRender()
Solution:
To help detect that we might have a custom component that has their own list of items, I introduced a new prop that allows the users to add in the length of their component that we'll consider when calculating the index and total set size of the elements in our list.
I also added 1 to the value that we're sending out to help adjust for the fact that aria-posinset is 1-based index and not 0 based-index.
Validation:
I tested the changes locally by using my changes with my own custom component with a list and validated that the numbers are all correct.