Async month changed callbacks #1052
Merged
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
|
This pull request is automatically deployed with Now. Latest deployment for this branch: https://material-ui-pic-git-fork-lindgren-linus-feature-async-ca-d139cc.mui-org.now.sh |
Codecov Report
@@ Coverage Diff @@
## next #1052 +/- ##
==========================================
+ Coverage 94.92% 95.07% +0.14%
==========================================
Files 52 52
Lines 1301 1319 +18
Branches 176 180 +4
==========================================
+ Hits 1235 1254 +19
+ Misses 48 47 -1
Partials 18 18
Continue to review full report at Codecov.
|
dmtrKovalenko
requested changes
May 21, 2019
| @@ -5,7 +5,7 @@ import DayWrapper from './DayWrapper'; | |||
| import CalendarHeader from './CalendarHeader'; | |||
| import EventListener from 'react-event-listener'; | |||
| import SlideTransition, { SlideDirection } from './SlideTransition'; | |||
| import { Theme } from '@material-ui/core'; | |||
| import { Theme, CircularProgress } from '@material-ui/core'; | |||
There was a problem hiding this comment.
Make path import instead of core
| @@ -136,12 +138,20 @@ export class Calendar extends React.Component<CalendarProps, CalendarState> { | |||
| this.props.onChange(utils.mergeDateAndTime(day, date), isFinish); | |||
| }; | |||
|
|
|||
| public handleChangeMonth = (newMonth: MaterialUiPickersDate, slideDirection: SlideDirection) => { | |||
| public handleChangeMonth = async ( | |||
There was a problem hiding this comment.
I am not sure that we need async/await here for one option. It will lead to some additional bundle size.
Use .then
| @@ -24,6 +23,40 @@ describe('Calendar', () => { | |||
| component.find('CalendarHeader').prop<any>('onMonthChange')(utilsToUse.date()); | |||
| expect(onMonthChangeMock).toHaveBeenCalled(); | |||
| }); | |||
|
|
|||
| it('Should not display loading indicator when synchronous', () => { | |||
There was a problem hiding this comment.
The tests must be moved to e2e. Or at least using mount.
There was a problem hiding this comment.
Thanks for the feeback!
I could definitely move the tests but I'm wondering if they really are e2e tests?
Except from the confusing test names (who clearly mentions visibility of elements, which is kindof misleading and should be changed), the tests really only checks so that the state of the component is changed properly.
Mount is probably more suitable for these kind of tests, I agree, but does state manipulation in components belong to e2e tests?
There was a problem hiding this comment.
Yeep, it is more integration then e2e.
We are on the road to testing public interface rather then internal component logic
There was a problem hiding this comment.
mount tests just named e2e for some reason
There was a problem hiding this comment.
Moved the tests and modified them now
cevr
reviewed
May 21, 2019
| <> | ||
| {(this.state.loadingQueue > 0 && ( | ||
| <div className={classes.progressContainer}> | ||
| <CircularProgress /> |
There was a problem hiding this comment.
Must this render a CircularProgress? I believe this should be an option or at the very least able to be replaced by the user. @dmtrKovalenko What do you think?
There was a problem hiding this comment.
Yep would be cool to pass it down from props.
There was a problem hiding this comment.
Good idea! I added support for an optional loadingIndicator property that will replace the CircularProgress if not undefined.
|
Thank you for the response! |
dmtrKovalenko
approved these changes
Jun 2, 2019
|
Sorry for long delay. Was pretty busy last week |
|
Thank you! I`ve been right now adding your branch as upstream to push it myself! |
dmtrKovalenko
added a commit
that referenced
this pull request
Jul 31, 2019
* Fix not working logo * Mention required material-ui v4 in README.md * 📝 improve grammar in docs (#1073) * 📝 improve grammar in docs First of all, amazing project! 🎉 I took the liberty, and read through the docs, and on my way, I corrected some of the English grammar, I hope you don't mind! Keep up the great work! 👨🏭 * 📝 more grammar improvements * Fix time order in RTL DateTimePicker (mui-org#1071) (#1072) * Consume fade correctly (#1077) * Remove react-event-listener (#1075) * Remove react-event-listener * Fix lint * Make reusable useKeyDown * Update packages | 02 06 2018 (#1079) * Update packages * Shut up now bot * Fix now.json silent location * Fix ts error for navigator.permissions * Disable ToolbarButtonOverride, closes #1076 (#1080) * Fix inputRef collision, closes #1069 (#1081) * [DatePicker] add strictCompareDates prop (#1067) * DatePicker: add validateStrict props Add strict camparison for minDate, maxDate, disablePast, disableFuture * Min/Max date validation rename props validateStrict to strictCompareDates * DateValidation: strictCompareDates reduce conditions * Async month changed callbacks (#1052) * Added async handling of onMonthChange with loading indicator * Only show loading indicator if onMonthChange returns a Promise * Added test cases * Added some more test cases. Testing push and pop functionality of loading queue * Removed unused import * Updated docs and changed callback type in MonthSelection * Replaced 'async/await' with 'then' to reduce bundle size * Importing CircularProgress by path instead * Added support for optional loading indicator as a property * Moved tests to e2e * Removed unused import * Fixed Prettier violation * [typescript] Fix incorrectly inferred type for keyboard onChange (#1083) * Support keyboardIcon for v3, closes #1084 (#1085) * Support keyboardIcon for v3. closes #1084 * Add new prop to the docs * Support TextFieldComponent prop (#1087) * v3.1.0 * Change release script * Change bundlephobia badge link * Compare date strict tests (#1089) * WrappedKeyboardPicker: send strictCompareDates prop * compareDateStrict: add tests * Fix "todayLabel" prop type description. (#1091) * Fix "todayLabel" prop type description. * Update lib/src/wrappers/ModalWrapper.tsx Co-Authored-By: Dmitriy Kovalenko <dmtr.kovalenko@outlook.com> * Fix typescript material-ui overrides example (#1092) * Spread emptyLabel from passing to textField, closes #1093 * Fix incorrect DateTimePicker year button alignment on wide devices * Inner state for picker modal (#1095) * Use inner state for displayed date in calendar * Make inline pickers controlled by default * Fix tests * Fix deadlock on rendering with usePickerState * Fix crashing on utils change * Fix not applying keydown listener, closes #1090 * Update packages 08.06.2019 (#1096) * Update packages 08.06.2019 * Fix prettier * [docs] Add fetching data example (#1097) * Use inner state for displayed date in calendar * Make inline pickers controlled by default * Fix tests * Fix deadlock on rendering with usePickerState * Fix crashing on utils change * [docs] Add fetching data example * Change text and remove unnecessary details from example * Adjust some props description * [Refactoring] Reorganize folder structure (#1098) * Regorganize folder structure * Create new views folder with all components * Remove unnecessary theme type annotations * Restore lost in merge changes * Fix imports in tests * [Refactoring] Refactor Calendar.tsx * [Refactoring] Better hook memoization (#1101) * [Refactoring] Refactor useOpenState implementation Eliminate code duplication between the controlled and the uncontrolled implementation. Albeit short today, the code that calls the onOpen/onClose callbacks was duplicated. The controlled variant did not memoize with useCallbacks(). Fix memoization by not depending on `props` as a whole. * [Refactoring] Better hook memoization in usePickerState/useKeyboardPickerState Avoid passing the entire `props` object into the hooks' `deps', as it will usually be a fresh object, which prevents memoization. * Make "Today" button to set inner state, instead of onChange * Fix prettier in-merge error * [#1111] Properly rely on views order when switching views (#1116) * Polyfill array includes manually for IE11 (#1117) * Polyfill array includes manually for IE11 * Make proper implementation of arrayIncludes 🙆♂️ * Update release script * v3.1.1 * Add visual regression tests (#1121) * Add visual regression tests * Make docs run in backgorund on ci for tests * Fix percy command not found error * Add cypress verify on install * Restory cypress binary cache before running * Add additional yarn install * Change order of builds * Fix tsconfig.json error * Add more visual regression scenarios * Update config.yml * Disable random icons in percy * Make year to be 2019 in tests * Add more scenarios * Completely hide Ads in percy * [docs] Fix crashing on changing utils (#1123) * [docs] Fix crashing on changing utils * Rewrite MuiPickersUtilsProvider on hooks * Fix moment localization example * Update SecondsTimePicker.example.jsx (#1125) * Add Percy badge (#1129) * [docs] Fix views type labels for DatePicker API (#1138) * [#1126] Fix rerendering loop * Fix invalidLabel prop-type warning * [#1126] Fix rerendering loop (#1139) * Custom spacing support (#1141) * Add custom theme spacing example * Fix issues with theme spacing * Allow to pass custom rifm formatter (#1142) * v3.1.2 * 3.1.2 * [Calendar] Remove arrows white background on sliding from disabled state (#1152) * Add Hijri calendar example (#1153) * Add Hijri calendar example * Combine jalaali and hijri in single page * Address PR feedback * Fix headers size * Do not throw error if input value was cleared symbol by symbol (#1160) * [TimePicker] Properly hide separator if views not included (#1161) * [InlineVariant] Do not update value on backdrop click (#1162) * [docs] Fix variant prop type displaying, highlight code in 'type' column (#1163) * Fix variant prop displaying, highlight type code * Remove additional space after each prop * Add variant="static" (#1164) * Add variant="static" * More variant="static" examples * Landscape orientation support (#1177) * Fix persian examples * Remove injected css after front-end mount * Determine and apply styles for current orientation * Add tests * Support landscape for TimePicker * Use isomorphic effect for event handler * Fix lint errors * Add new props to the documentation * Fix displaying short weekdays in datepicker toolbar * Custom toolbar component (#1182) * Add new ToolbarComponent prop for custom toolbars * Update props docs * Run docgen on precommit hook * Run docgen on precommit hook * Run git add after precommit command * Automatically format json before prettier * Do not accept a and p sympbols in not am/pm mode (#1183) * Fix keyboard popover position (#1184) * Correct position of inline wrapper popover * Adjust variant prop description * Adjust props description * Uncomment example * Add prop-types.json to prettier-ignore * Properly freeze clock for percy * Use yarn to publish * v3.2.0 * Update publish command * Use KeyboardDatePicker for formik integration example * [docs] Add redux form integration example (#1189) * install redux and redux-form * configure redux-form * wrap App with redux provider * create redux form example * add react-redux types to dev dependencies * Update docs/pages/guides/form-integration.mdx Co-Authored-By: Dmitriy Kovalenko <dmtr.kovalenko@outlook.com> * move redux to example folder * Wrap example with redux * remove redux-thunk * modify onChange to allow empty field in the other way, the field always gotten the last valid value, even when explicit deleted. This way, the user can clear the field! * move store to inside Form example * Update useIsLandscape.tsx (#1199) Fix crashing on changing orientation for some devices * Update CSS overrides example (#1201) isSelected is no longer a valid theme entry for MUI Pickers, so update the docs to reflect this change. Also extends the example for dayDisabled override. * Support auto detection of orientation change for safari (#1207) * Add inputProps to the outputted typescript definitions (#1208) * Fix dispatching 2 onChange events with variant="inline" (#1209) * Fix dispatching 2 onChange events with variant="inline" * Update lib/src/_shared/hooks/usePickerState.ts * Make autoOk property to have bigger prioritiy * Add releases page (#1210) * Add releases page It lists available documentation websites for v3+ versions * Use constant for the latest url link * Make releases table horisontally scrollable on mobile * Fix grammar * Add shorthand for static state managment (#1212) * [KeyboardDateInput] Fix not updating formatter when new mask passed (#1213) * fix when both clear button & showToday button (#1211) * Add babel transpilation to esm (#1206) * Add babel transpilation to esm * Use babel for rollup as well * Fix missing * in jsdoc comment for ToolbarComponent * Update packages (#1217) * Update packages * Fix overrides typing * Add { force: true } for readonly inputs * Add one more { force: true } * v3.2.1 * Remove old versions from readme * More clearly highlight requiring version of date-fns
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.
Description
I found a need for fetching date validation information asynchronously from a server and thought that onMonthChange would be the perfect callback function for fetching more information about each month.
The property onMonthChange can now return a Promise (but can still return void, so this change won't break anything) and the Calendar component will await the callback while displaying a loading indicator.