-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Jolore/calendar updates #4759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jolore/calendar updates #4759
Conversation
…c-react into jolore/CalendarUpdates
| } | ||
|
|
||
| private _onSelectMonthKeyDown = (index: number): (ev: React.KeyboardEvent<HTMLElement>) => void => { | ||
| return (ev: React.KeyboardEvent<HTMLElement>) => this._onKeyDown(() => this._onSelectMonth(index), ev); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want both ENTER and SPACE because otherwise onClick will work for both mouse and SPACE since its a <button/>. I ask because I recently fixed a semi-related bug where buttons were firing their handlers twice #4662.
If we continue with this approach, I do think we can reduce this function signature by at least eliminating one closure, but I need to try myself with code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the SPACE issue, good call, that was an unintentional bug. Should be fixed now. Removed the "|| key.space" in the callback so that space will only fire the onClick callback
| } from '../../Utilities'; | ||
| import { compareDates, compareDatePart } from '../../utilities/dateMath/DateMath'; | ||
| import * as stylesImport from './DatePicker.scss'; | ||
| import { FocusTrapZone } from 'office-ui-fabric-react/lib/FocusTrapZone'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import should be a relative path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed now, thanks for helping figure this out David
…ly to calendar" This reverts commit 6bdc494.
* fixing focusZone algnment issue * calendar styling and functionality updates * undoing focusZone change * initial commit * making all the styling updates Hiroshi requested * adding change file * undoing focustrapzone change, updating snapshots * implementing the rest of the styling changes from Hiroshi's designs * fixing issue where navigating was not highlighting month picker correctly * updating snapshots * fixing issue where enter key didn't work in month picker * updating styling, fixing focus and focus trap bugs in datepicker and calendar component * fixing focus borders * adding change file * undoing unnecessary example change * fixing bug with space key double selecting * removing old change file * fixing static import to relative import * fixing bug when quickly moving in and out with the mouse * fixing the rounded corners from datePicker applying incorrectly to calendar * Revert "fixing the rounded corners from datePicker applying incorrectly to calendar" This reverts commit 6bdc494. * adding back fix for quickly mousing in and out of component
Pull request checklist
$ npm run changeDescription of changes
Updating the rest of the styling for the calendar component to latest design guidelines. Also fixing some bugs in Calendar and DatePicker.
Fixing one of the bugs referenced in #3300, with Datepicker's disableAutoFocus not preventing focus after click. Not resolving it since it was a more expansive feature request.
Fixes #4415 by wrapping the date picker in a focusTrapZone, making it so that focus stays inside while tabbing and goes back to beginning or end. DatePicker will close on ESC key or after selecting a date.
Fixes #4623 by removing the custom logic for resetting focus on dismiss of the date picker. Now we rely on FocusTrapZone to reset the focus back where it should go when the date picker closes, which resolves the issue.
inside MSFT can see this link for updates: http://jolore/fabric/#/examples/calendar
Focus areas to test
(optional)