Add DatePicker component #5509

kidroca · 2021-09-24T19:23:59Z

Details

Adds a native datepicker per each platform

Fixed Issues

Tests

Go to a place that uses a DatePicker (Reimbursement Account Page)
- IdentityForm: BeneficialOwner step, Requestor step,
- Company Step
Selecting a date field should bring a datepicker popup (ios/android)
- on web/desktop we have a native <input type=date />. It supports both typing and popping a calendar picker
- ios displays a spinner date picker
- android displays a calendar date picker
Selecting a date from the picker fills the date in the form
Submitting the form should submit the correct value to the backend

⚠️ I couldn't submit any of these forms as I don't know how - I don't have a US bank account that I can register
I've made sure dates are sent as strings in the format YYYY-MM-DD and inspected that the submit payload contains the value

QA Steps

Same as above

Tested On

Screenshots

Web

Chrome

Screen.Recording.2021-09-24.at.22.44.52.mov

Safari

Screen.Recording.2021-09-24.at.22.50.37.mov

Firefox

New.Expensify.Mozilla.Firefox.2021-09-24.23-06-15.mp4

Mobile Web

iOS (Safari)

Screen.Recording.2021-09-27.at.10.25.55.mov

Android (Chrome)

Android.Emulator.-.Pixel_2_API_29_5554.2021-09-24.23-09-18.mp4

Desktop

Screen Recording 2021-09-24 at 22.53.19

iOS

Screen.Recording.2021-09-24.at.23.29.57.mov

Android

Android.Emulator.-.Pixel_2_API_29_5554.2021-09-24.23-02-40.mp4

For example the DatePicker sets the value in a text field without using the keyboard - the value is picked from a picker

Added accent as it's used for the datepicker background Moved colors to colors.xml

On desktop and web the browser populates a pattern and a datepicker icon

Noticed the native picker on web has the word "reset" And the behavior to return to the previous value instead of closing the picker

Pressing inside the field (e.g. not on the label) is not activated without this

roryabraham · 2021-09-27T22:00:52Z

@Expensify/design The color choices for the Android calendar picker (see last screen recording) don't feel on-brand. Thoughts?

deetergp

The default date template conflicts with the placeholder in Web. Let's nudge the placeholder/title up a few pixels so they don't overlap.

src/components/DatePicker/datepickerPropTypes.js

src/components/ExpensiTextInput/baseExpensiTextInputPropTypes.js

roryabraham

Overall this implementation seems good to me. Didn't do much testing though

src/components/DatePicker/datepickerPropTypes.js

src/components/DatePicker/index.android.js

src/components/DatePicker/index.js

src/components/DatePicker/index.ios.js

src/components/DatePicker/index.js

roryabraham · 2021-09-27T22:53:33Z

src/components/DatePicker/index.js

+    componentDidMount() {
+        if (this.inputRef) {
+            // Adds nice native datepicker on web/desktop. Not possible to set this through props
+            this.inputRef.type = 'date';


This feels like a hack/code smell to me, but I don't have a better suggestion atm. Seems like it works so we can go with it for now.

Yep, it's hackish ⌨️ but allows us to reuse ExpensiTextInput
Other options are:

don't set the type to "date" and not have the native picker for web/mWeb/desktop.

use html elements (since we're on a web/desktop file) and return <input type=date />. But we'll have to refactor or re-code all the styles to match ExpensiTextInput

I did a quick search for other ways to pass this attribute to the native element and didn't find anything

If it makes sense to refine this in the future, we can swap a web specific library (like react-datepicker) and provide a uniform datepicker experience

src/components/DatePicker/index.js

We're using ExpensiInput which override some styles and render the input differently There's no dropdown arrow. When we focus the empty datepicker field it's only highlighted in blue. The user needs to tap again in order to see the native datepicker To avoid this confusion we're showing the datepicker immediately

# Conflicts: # src/pages/ReimbursementAccount/IdentityForm.js

On safari the datepicker would show a done and reset buttons We're also passing the locale to the datepicker, because we're partially localizing the modal ourselves it will be weird if the buttons are shown in one locale while the month names in another. RNDatepicker respects the locale only on iOS

kidroca

Added notes on the last changes

kidroca · 2021-09-28T19:09:42Z

src/components/DatePicker/index.js

+    /**
+     * Pops the datepicker up when we focus this field. This only works on mWeb
+     * On mWeb the user needs to tap on the field again in order to bring the datepicker. But our current styles
+     * don't make this very obvious. To avoid confusion we open the datepicker when the user focuses the field
+     */
+    showDatepicker() {
+        if (this.inputRef) {
+            this.inputRef.click();
+        }
+    }


This was mainly added due to very bad UX on mWeb Android (Chrome).
Tapping on the field would first highlight it. The datepicker would appear if you tap on the input element but due to our styles (from ExpensiInput the input wrapped in a <div>) the target area is actually smaller

Before

Android.Emulator.-.Pixel_2_API_29_5554.2021-09-28.21-22-55.mp4

it might not be obvious but the picker would appear only when we tap below the field label near the middle of the field. And only after we've focused the field...

After

Android.Emulator.-.Pixel_2_API_29_5554.2021-09-28.21-24-08.mp4

The picker appears as soon as we tap the field (similar to how it works on Android native)
Tested the rest of the platforms and there aren't any issues

kidroca · 2021-09-28T19:18:07Z

web/index.html

+        input[type=date]::-webkit-calendar-picker-indicator {
+            position: absolute;
+            right: 12px;
+            top: 16px;
+        }


This positions the datepicker calendar icon similar to how we do it on other fields (select picker)

Edge:

Chrome:

Firefox and Safari are not displaying an icon:

kidroca · 2021-09-28T19:22:49Z

@deetergp regarding:

In order to test this on iOS, I had to manually install react-native-iphone-x-helper. Is that a requirement of this PR? If so please add it.

...

I'm going to merge the latest changes from main to fix a conflict an the package discrepancy should be fixed as well

After merging latest main I've experienced the same problem, it's seems to be related to cache. The metro bundler suggested to reset it and indeed after clearing cache there were no issues for me

If this error happens again, try this, stop the RN metro server and start it again like this:

npm start -- --reset-cache

kidroca · 2021-09-28T19:23:00Z

Changes are ready for review

deetergp

Code changes look good, but I have one last nitpick: that the Incorporation Date appears center justified in mWeb on iOS at Workspace > Get Started > Connect Manually

The styles are not getting applied in storybook because they were in web/index.html To avoid such problems let's move them to the datepicker that uses them Using a `.css` file is a special case here, because it's impossible to target pseudo-elements from js code (e.g. ::-webkit)

kidroca

Notes on the latest changes

Code changes look good, but I have one last nitpick: that the Incorporation Date appears center justified in mWeb on iOS

This should be fixed

kidroca · 2021-09-29T12:14:10Z

src/components/DatePicker/styles.css

+.expensify-datepicker::-webkit-calendar-picker-indicator {
+    position: absolute;
+    right: 12px;
+    top: 13px;
+}
+.expensify-datepicker::-webkit-date-and-time-value {
+    text-align: left;
+}


Noticed these styles weren't getting applied in storybook so I moved them here - in a .css file

Using a .css file is a special case here, because it's impossible to target pseudo-elements from js code (e.g. ::-webkit)

It also keeps things together. It was a bit weird for me to have these defined in index.html

Please add a code comment explaining this 😄

kidroca · 2021-09-29T12:15:25Z

src/stories/Datepicker.stories.js

+export default {
+    title: 'Components/Datepicker',
+    component: DatePicker,
+    argTypes: {
+        onChange: {action: 'date changed'},
+    },
+    args: {
+        value: '',
+        label: 'Select Date',
+        placeholder: 'Date Placeholder',
+        errorText: '',
+        hasError: false,
+    },
+};


Added a storybook sample as per: #5509 (comment)

cc @roryabraham

deetergp

Looks good—-thank you for the changes.

roryabraham · 2021-09-29T19:23:51Z

src/components/DatePicker/index.ios.js

+Datepicker.propTypes = datepickerPropTypes;
+Datepicker.defaultProps = defaultProps;
+
+export default withLocalize(Datepicker);


NAB but maybe useful to mention somewhere that iOS is the only platform where we have to manage localization ourselves, and on all others it's controlled by the OS

I'll open a separate request to put a note about this

We're localizing the "Reset" and "Done" button that we put on our own popup.
RNDatepicker would be shown in the system locale. I'm passing the locale to RNDatepicker because it will be weird to have the "Done" and "Reset" buttons in one locale and the picker months in another - it can happen

On Android the buttons and the modal are handled by the system so they are always in the system locale

roryabraham · 2021-09-29T19:24:58Z

src/components/DatePicker/styles.css

+.expensify-datepicker::-webkit-calendar-picker-indicator {
+    position: absolute;
+    right: 12px;
+    top: 13px;
+}
+.expensify-datepicker::-webkit-date-and-time-value {
+    text-align: left;
+}


Please add a code comment explaining this 😄

@deetergp

I'm not explicitly approving because I don't have time to test atm, but I'm removing my request for changes. @deetergp feel free to move forward on this PR without me if you've tested and feel comfortable

OSBotify · 2021-09-29T23:26:08Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-09-30T16:47:23Z

🚀 Deployed to staging by @deetergp in version: 1.1.3-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2021-10-04T19:16:54Z

🚀 Deployed to production by @chiragsalian in version: 1.1.4-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

kidroca added 17 commits September 24, 2021 18:45

ExpensiTextInput: add support for non controlled input

37135b9

ExpensiTextInput: add forceActiveLabel prop

125894a

Create Datepicker component for web & desktop

f8ac6b6

Create Datepicker component for ios

c1922b8

RequestorStep format dob string

a2c9fa5

Include Datepicker on Company and Identity forms

5226357

ExpensiTextInput - handle programatic value uptates

03ad0e5

For example the DatePicker sets the value in a text field without using the keyboard - the value is picked from a picker

DatePicker: local state and handling

c149599

DatePicker: Use "Confirm" instead of "Save"

9f647d5

DatePicker: Android picker

b29b1c6

Refactor add onPress handler to BaseExpensiTextInput

215a925

DatePicker update Android handling

3173150

Android theming

014e4f8

Added accent as it's used for the datepicker background Moved colors to colors.xml

DatePicker: Extract common datepicker prop types

52beb8d

DatePicker: handle disabled state

e0259b6

ExpensiTextInput: Add default for value

374f3a3

Update date prop type for IdentityForm

73fbf05

kidroca force-pushed the kidroca/datepicker branch from 4eeaef1 to 73fbf05 Compare September 24, 2021 19:49

kidroca added 2 commits September 24, 2021 23:20

Force active label only for large screens

0b2e111

On desktop and web the browser populates a pattern and a datepicker icon

DatePicker: replace discard with reset

6eb8946

Noticed the native picker on web has the word "reset" And the behavior to return to the previous value instead of closing the picker

kidroca marked this pull request as ready for review September 24, 2021 20:37

kidroca requested a review from a team as a code owner September 24, 2021 20:37

MelvinBot requested review from deetergp and removed request for a team September 24, 2021 20:37

ExpesniTextInput: add onPressOut handler

3d15dd8

Pressing inside the field (e.g. not on the label) is not activated without this

roryabraham self-requested a review September 27, 2021 21:16

roryabraham assigned kidroca Sep 27, 2021

deetergp reviewed Sep 27, 2021

View reviewed changes

src/components/DatePicker/datepickerPropTypes.js Outdated Show resolved Hide resolved

src/components/ExpensiTextInput/baseExpensiTextInputPropTypes.js Outdated Show resolved Hide resolved

roryabraham previously requested changes Sep 27, 2021

View reviewed changes

kidroca added 2 commits September 28, 2021 20:28

DatePicker: jsdocs parameters

9db7aa9

kidroca force-pushed the kidroca/datepicker branch from 3220f91 to 9db7aa9 Compare September 28, 2021 18:37

kidroca added 2 commits September 28, 2021 21:38

Merge branch 'main' into kidroca/datepicker

71a8c32

# Conflicts: # src/pages/ReimbursementAccount/IdentityForm.js

kidroca commented Sep 28, 2021

View reviewed changes

kidroca requested review from deetergp and roryabraham September 28, 2021 19:23

deetergp requested changes Sep 28, 2021

View reviewed changes

kidroca added 4 commits September 29, 2021 13:59

DatePicker: make Safari date alignment to the left

dfb3d17

Storybook: add datepicker story

61f0f61

Pass the hasError prop to the datepicker text field

859e196

kidroca commented Sep 29, 2021

View reviewed changes

kidroca requested a review from deetergp September 29, 2021 12:23

deetergp approved these changes Sep 29, 2021

View reviewed changes

roryabraham reviewed Sep 29, 2021

View reviewed changes

deetergp merged commit ce056a4 into Expensify:main Sep 29, 2021

kidroca deleted the kidroca/datepicker branch September 30, 2021 10:43

flodnv mentioned this pull request Sep 30, 2021

[HOLD] Revert "Add DatePicker component" #5599

Closed

OSBotify mentioned this pull request Sep 30, 2021

Deploy Checklist: New Expensify 2021-09-30 #5605

Closed

15 tasks

kidroca mentioned this pull request Oct 4, 2021

[No QA] Add clarification notes on datepicker #5627

Merged

5 tasks

ntdiary mentioned this pull request Jan 13, 2023

[HOLD] [$4000] DatePicker doesn't have green border while choosing the date. #14140

Closed

1 task

sobitneupane mentioned this pull request Jan 20, 2023

[$1000] Date picker is not centered and has too small clickable area #12957

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DatePicker component #5509

Add DatePicker component #5509

kidroca commented Sep 24, 2021 •

edited

Loading

roryabraham commented Sep 27, 2021

deetergp left a comment

roryabraham left a comment •

edited

Loading

roryabraham Sep 27, 2021

kidroca Sep 28, 2021 •

edited

Loading

kidroca left a comment

kidroca Sep 28, 2021 •

edited

Loading

kidroca Sep 28, 2021

kidroca commented Sep 28, 2021

kidroca commented Sep 28, 2021

deetergp left a comment

kidroca left a comment •

edited

Loading

kidroca Sep 29, 2021

roryabraham Sep 29, 2021

kidroca Sep 29, 2021

deetergp left a comment

roryabraham Sep 29, 2021

kidroca Sep 30, 2021

roryabraham Sep 29, 2021

OSBotify commented Sep 29, 2021

OSBotify commented Sep 30, 2021

OSBotify commented Oct 4, 2021

Add DatePicker component #5509

Add DatePicker component #5509

Conversation

kidroca commented Sep 24, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Chrome

Safari

Firefox

Mobile Web

iOS (Safari)

Android (Chrome)

Desktop

iOS

Android

roryabraham commented Sep 27, 2021

deetergp left a comment

Choose a reason for hiding this comment

roryabraham left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kidroca Sep 28, 2021 • edited Loading

Choose a reason for hiding this comment

kidroca left a comment

Choose a reason for hiding this comment

kidroca Sep 28, 2021 • edited Loading

Choose a reason for hiding this comment

Before

After

Choose a reason for hiding this comment

kidroca commented Sep 28, 2021

kidroca commented Sep 28, 2021

deetergp left a comment

Choose a reason for hiding this comment

kidroca left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deetergp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OSBotify commented Sep 29, 2021

OSBotify commented Sep 30, 2021

OSBotify commented Oct 4, 2021

kidroca commented Sep 24, 2021 •

edited

Loading

roryabraham left a comment •

edited

Loading

kidroca Sep 28, 2021 •

edited

Loading

kidroca Sep 28, 2021 •

edited

Loading

kidroca left a comment •

edited

Loading