Fix Calendar component PREV/NEXT month, year, and "Go to today" handlers firing twice#4662
Merged
lorejoh12 merged 2 commits intomicrosoft:masterfrom Apr 25, 2018
Conversation
…ck handles space with button nodes.
jordandrako
approved these changes
Apr 24, 2018
lorejoh12
approved these changes
Apr 25, 2018
Markionium
added a commit
to Markionium/office-ui-fabric-react
that referenced
this pull request
Apr 26, 2018
* master: (42 commits) Applying package updates. ProgressIndicator: Finish conversion to mergeStyles (microsoft#4595) Fix props validation for Breadcrumb (microsoft#4666) No unused vars part of ts (microsoft#4670) Picker/Autofill: fixes several minor bugs. (microsoft#4569) Fix Calendar component PREV/NEXT month, year, and "Go to today" handlers firing twice (microsoft#4662) Applying package updates. Merge styles order (microsoft#4664) Fabric component: revert class change and make it backwards compatible (microsoft#4671) Addressing Issue microsoft#3707 - OverflowSet: Add the ability to set aria-label (microsoft#4667) Fix input type for Tile ARIA label prop (microsoft#4668) Fix theme slots for DetailsList header colors (microsoft#4658) Applying package updates. Jolore/calendar updates (microsoft#4643) Remove wordWrap setting. (microsoft#4657) Pivot: convert to mergeStyles - part 1 (microsoft#4656) Use the `data-is-scrollable` attribute on the correct ScrollablePane div (microsoft#4602) Applying package updates. Remove unused iconClassName prop from Nav.types (microsoft#4634) Jest snapshots: classes in animations should autoexpand. (microsoft#4647) ...
Markionium
added a commit
to Markionium/office-ui-fabric-react
that referenced
this pull request
Apr 26, 2018
* master: (34 commits) Applying package updates. ProgressIndicator: Finish conversion to mergeStyles (microsoft#4595) Fix props validation for Breadcrumb (microsoft#4666) No unused vars part of ts (microsoft#4670) Picker/Autofill: fixes several minor bugs. (microsoft#4569) Fix Calendar component PREV/NEXT month, year, and "Go to today" handlers firing twice (microsoft#4662) Applying package updates. Merge styles order (microsoft#4664) Fabric component: revert class change and make it backwards compatible (microsoft#4671) Addressing Issue microsoft#3707 - OverflowSet: Add the ability to set aria-label (microsoft#4667) Fix input type for Tile ARIA label prop (microsoft#4668) Fix theme slots for DetailsList header colors (microsoft#4658) Applying package updates. Jolore/calendar updates (microsoft#4643) Remove wordWrap setting. (microsoft#4657) Pivot: convert to mergeStyles - part 1 (microsoft#4656) Use the `data-is-scrollable` attribute on the correct ScrollablePane div (microsoft#4602) Applying package updates. Remove unused iconClassName prop from Nav.types (microsoft#4634) Jest snapshots: classes in animations should autoexpand. (microsoft#4647) ...
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.
Pull request checklist
$ npm run changeDescription of changes
In #3741 we refactored all the Calendar
<span/>nodes to<button/>, which was a good change 👍 . However, React invokesonClickforKeyboardEventif thekeyCodeisSPACE. Since each<button/>node had anonClickandonKeyDownit would invoke the PREV/NEXT month, year, and also "Go to today" callbacks twice.Solution Options:
I went with # 1, but there's an alternative solution:
ENTERto invoke the<button/>then the change is what is in this PR. We need to only invoke theonKeyDowncallbacks if the key wasENTER.SPACEto invoke the callback then we can safely remove theonKeyDownhandlers.Since # 2 would would change prior behavior so I left went with option # 1.
Focus areas to test
Note: that the UI doesn't update when "Go to today" is invoked. Looks like a preexisting bug on
masterthat I'll look into but may file separately. I assume the UI updated at one point, but need to check behavior in prior commits.