OverflowSet: Add Focus(with forceIntoFirstChild) and focusElement() functions#4485
Merged
jspurlin merged 3 commits intomicrosoft:masterfrom Apr 9, 2018
Merged
Conversation
jspurlin
changed the title
christiango
changed the title
christiango
approved these changes
Apr 7, 2018
ddlbrena
reviewed
Apr 8, 2018
| /** | ||
| * Sets focus to the button. | ||
| */ | ||
| focus: () => void; |
Contributor
There was a problem hiding this comment.
focus [](start = 2, length = 5)
is removing this not a breaking change? does CommandBar.base.tsx not need to be updated? #Closed
Contributor
There was a problem hiding this comment.
ups never mind, i guess that is why the parameter above is optional
In reply to: 179932909 [](ancestors = 179932909)
ddlbrena
reviewed
Apr 8, 2018
| * Sets focus to a specific child element within the zone. This can be used in conjunction with | ||
| * onBeforeFocus to created delayed focus scenarios (like animate the scroll position to the correct | ||
| * location and then focus.) | ||
| * @param {HTMLElement} element The child element within the zone to focus. |
Contributor
There was a problem hiding this comment.
element [](start = 26, length = 7)
[nit] childElement or rename parameter to element to match implementation
ddlbrena
reviewed
Apr 8, 2018
| * @param {HTMLElement} element The child element within the zone to focus. | ||
| * @returns True if focus could be set to an active element, false if no operation was taken. | ||
| */ | ||
| public focusElement(element: HTMLElement): boolean { |
Contributor
There was a problem hiding this comment.
element [](start = 22, length = 7)
does the parameter here not need to be marked as optional the to match the interface?
micahgodbolt
approved these changes
Apr 9, 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.
4 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.
Pull request checklist
$ npm run changeDescription of changes
Before OverflowSets didn't expose the goodness of being able to focus vs focus (firstChild), or a specific child element via foucsElement. This exposes the functionality that FocusZone has (and implements it for when it is not in a FocusZone)
Focus areas to test
Verified that this works as expected for both the case when the OverflowSet contains a FocusZone and when does not and everything works as expected