Refactor Datepicker to work with Form.js#8770
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
| showPicker(event) { | ||
| this.setState({isPickerVisible: true}); | ||
| event.preventDefault(); | ||
| } |
There was a problem hiding this comment.
There was a lint warning saying that showPicker should come after setDate, that's why I moved this.
|
Assigning @Luke9389 since you were assigned to the original PR. |
tylerkaraszewski
left a comment
There was a problem hiding this comment.
I'm far from an expert in any of this code but I left a couple comments.
Luke9389
left a comment
There was a problem hiding this comment.
just a lil NAB
Otherwise lookin good
|
Updated! |
tylerkaraszewski
left a comment
There was a problem hiding this comment.
Looks good based on my limited knowledge. :)
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Cherry-picked to staging by @sketchydroide in version: 1.1.57-8 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
🚀 Deployed to production by @chiragsalian in version: 1.1.57-17 🚀
|
Details
Refactors
Datepickerto work with the new Form component.Fixed Issues
$ #7536
Tests
+ > New WorkspaceConnect bank accountand go through the flow, using the information in this SO.Company step, perform the following actions:Save & continueand verify that theIncorporation datefield shows an error.Incorporation datewith a date and verify that the error goes away.Company stepand verifying that the previously entered date is still there.Save & continue.Date of birthin thePersonal informationstep.npm run storybookand run the checks from step 3.PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followed/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
+ > New WorkspaceConnect bank accountand go through the flow, using the information in this SO.Company step, perform the following actions:Save & continueand verify that theIncorporation datefield shows an error.Incorporation datewith a date and verify that the error goes away.Company stepand verifying that the previously entered date is still there.Save & continue.Date of birthin thePersonal informationstep.Screenshots
Web
web.mov
Mobile Web
mweb.mov
Desktop
desktop.mov
iOS
ios.mov
Android
android.mov