Remove explicit blocking of Alt key for keytip start sequence #13389
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
|
When I run "yarn change" from the root it shows that I have changes in like 20 packages, and that there is also already a change file for office-ui-fabric-react. Not sure what I should do here? yarn clean maybe? |
|
@kelseyyoung You probably need to update the URL for your |
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Perf comparison
Perf tests with no regressions
|
Asset size changes
Baseline commit: cfcf5178fb8304109274a68121c7762a91f55d5b (build) |
|
@ecraig12345 any chance you could take a look at this? |
|
@kelseyyoung I don't really know anything about keytips and their expected behavior--so maybe get @jspurlin to review instead? (looks like he's worked on this in the past) |
jspurlin
approved these changes
Jun 1, 2020
|
🎉 Handy links: |
miroslavstastny
pushed a commit
to levithomason/fluentui
that referenced
this pull request
Jun 8, 2020
…oft#13389) * Remove explicit blocking of Alt key * Change files
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.
Pull request checklist
$ yarn changeDescription of changes
When writing keytips initially we had blocked the recognition of the "Alt" key because that is reserved for putting focus on the top bar of the browser. However there are users of Keytips that are writing their app in an Electron application that looks and behaves like a native desktop app. In native desktop apps the default key for keytips is just "Alt", and because they don't run in a traditional browser they won't have any issues using it
Focus areas to test
Tested that with removing these lines that you can specify "Alt" to start your keytips. If done in a traditional browser this will "break" in the sense that keytips will appear but focus will also go to the browser top bar, meaning the user can't interact with keytips