Skip to content
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

[$500] Update Datepicker UX #52621

Open
dannymcclain opened this issue Nov 15, 2024 · 92 comments
Open

[$500] Update Datepicker UX #52621

dannymcclain opened this issue Nov 15, 2024 · 92 comments
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

Comments

@dannymcclain
Copy link
Contributor

dannymcclain commented Nov 15, 2024

Problem

Currently our datepicker inputs always show the calendar popover, and this leads to a pretty cumbersome interface in places like the search filters where we have both a before and after date input.

CleanShot 2024-11-15 at 08 54 52@2x

Solution

Coming from this thread, let’s update our datepicker so that the calendar popover only appears when the input is focused.

CleanShot.2024-11-15.at.08.49.24.mp4

image

Notes:

  • For this update, we’re going to leave the placeholder text as is. Focusing the input should only change the border to green and make the popover appear. (It should not bring up the keyboard on mobile.)
  • Selecting a date in the popover should set the date in the input and blur the input (thereby dismissing the calendar popover).
  • We also need to update the selected state of the date to match our designs (green background, bold text, with the same text color we use for our green buttons).
  • For the date filter in search and other places where there may be multiple inputs, we do not want to auto-focus the first date input (as this would hide the input below it), but in situations where there is only one date input (like clearing your status for example) we do want to autofocus the input to reveal the popover right away.
  • We plan on doing a “phase 2” update to the datepicker where we will allow users to select a date by either using the popover OR by typing a date into the input, likely using some kind of guided input (example), so let’s keep that in mind as we’re making this update. But to be clear, for phase 1, we’re not planning on tackling the guided input aspect.

Figma file

cc @Expensify/design

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021857439292509917630
  • Upwork Job ID: 1857439292509917630
  • Last Price Increase: 2025-01-02
  • Automatic offers:
    • shubham1206agra | Contributor | 105277860
Issue OwnerCurrent Issue Owner: @thesahindia
@dannymcclain dannymcclain added Weekly KSv2 Improvement Item broken or needs improvement. labels Nov 15, 2024
@dannymcclain dannymcclain self-assigned this Nov 15, 2024
@dannymcclain dannymcclain added the External Added to denote the issue can be worked on by a contributor label Nov 15, 2024
@melvin-bot melvin-bot bot changed the title Update Datepicker UX [$250] Update Datepicker UX Nov 15, 2024
Copy link

melvin-bot bot commented Nov 15, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021857439292509917630

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 15, 2024
Copy link

melvin-bot bot commented Nov 15, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 15, 2024
@dannymcclain
Copy link
Contributor Author

@JmillsExpensify any opinion on which project makes the most sense for this? I was thinking #retain but I will defer to the smart people!

@Pholenk
Copy link

Pholenk commented Nov 15, 2024

Hi @dannymcclain I can't see the image and video attached. Can you reupload it, please?

@neonbhai
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Update Datepicker UX

What is the root cause of that problem?

Feature Request

What changes do you think we should make in order to solve the problem?

We will:

  1. Wrap the Date Popover in a Modal

    • In DatePicker.tsx, wrap the date popover here with a <Modal> tag:
      <View
      style={[styles.datePickerPopover, styles.border]}
      collapsable={false}
      >
      <CalendarPicker
      minDate={minDate}
      maxDate={maxDate}
      value={selectedDate}
      onSelected={onSelected}
      />
      </View>
    • The modal will have no backdrop and will appear only when the TextInput is focused.
  2. Handle Focus State

    • Since we need the onFocus & onBlur prop added by InputWrapper, we will use the datePicker from a component that wraps it in InputWrapper
    • Introduce an isDatePopoverVisible state to toggle the popover visibility based on onFocus and onBlur events.
         onFocus={() => setIsDatePopoverVisible(true)}
         onBlur={() => setIsDatePopoverVisible(false)}
  3. Autofocus Behavior

    • For date filters (e.g., in search), we will avoid auto-focusing the first date input to prevent hiding subsequent inputs using a prop shouldAutofocus
    • shouldAutofocus will betrue by default

@dannymcclain
Copy link
Contributor Author

@Pholenk Just re-uploaded! Hopefully that worked. Those are pretty important to be able to see 😂

@daledah
Copy link
Contributor

daledah commented Nov 15, 2024

Edited by proposal-police: This proposal was edited at 2024-11-16 08:52:21 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

  • New feature

What changes do you think we should make in order to solve the problem?

  1. Base code change in DatePicker:
function DatePicker(
    {
        containerStyles,
        defaultValue,
        disabled,
        errorText,
        inputID,
        label,
        maxDate = setYear(new Date(), CONST.CALENDAR_PICKER.MAX_YEAR),
        minDate = setYear(new Date(), CONST.CALENDAR_PICKER.MIN_YEAR),
        onInputChange,
        onTouched,
        placeholder,
        value,
        shouldSaveDraft = false,
        formID,
    }: DatePickerProps,
    ref: ForwardedRef<BaseTextInputRef>,
) {
    const styles = useThemeStyles();
    const {translate} = useLocalize();
    // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
    const [selectedDate, setSelectedDate] = useState(value || defaultValue || undefined);
    const {shouldUseNarrowLayout} = useResponsiveLayout();

    useEffect(() => {
        // Value is provided to input via props and onChange never fires. We have to save draft manually.
        if (shouldSaveDraft && !!formID) {
            FormActions.setDraftValues(formID, {[inputID]: selectedDate});
        }

        if (selectedDate === value || !value) {
            return;
        }

        setSelectedDate(value);
    }, [formID, inputID, selectedDate, shouldSaveDraft, value]);

+    const show = useRef<() => void>();
+    const onSelected = (newValue: string) => {
+        onTouched?.();
+        onInputChange?.(newValue);
+        setSelectedDate(newValue);
+        hideTooltipRef.current?.();
+        ref?.current?.blur();
+    };
+    const hideTooltipRef = useRef<() => void>();

+    const [containerInputWidth, setContainerInputWidth] = useState();

+    const renderTooltipContent = useCallback(() => {
+        return (
+            <View
+                style={[styles.datePickerPopover, styles.border, {width: containerInputWidth, pointerEvents: 'auto', zIndex: +10000}]}
+                collapsable={false}
+            >
+                <CalendarPicker
+                    minDate={minDate}
+                    maxDate={maxDate}
+                    value={selectedDate}
+                    onSelected={onSelected}
+                />
+            </View>
+        );
+    }, [containerInputWidth, minDate, maxDate, selectedDate, onSelected]);

    return (
+        <View style={styles.datePickerRoot}>
+            <GenericTooltip
+                shouldUseOverlay
+                wrapperStyle={{backgroundColor: 'transparent'}}
+                renderTooltipContent={renderTooltipContent}
+                // eslint-disable-next-line react/jsx-props-no-spreading
+                anchorAlignment={{horizontal: 'center', vertical: 'top'}}
+            >
+                {({showTooltip, hideTooltip, updateTargetBounds}) => {
+                    hideTooltipRef.current = hideTooltip;
+                    return (
+                        <View
+                            onLayout={(e: LayoutChangeEventWithTarget) => {
+                                // e.target is specific to native, use e.nativeEvent.target on web instead
+                                const target = e.target || e.nativeEvent.target;
+                                show.current = () => measureTooltipCoordinate(target, updateTargetBounds, showTooltip);
+                            }}
+                            style={[shouldUseNarrowLayout ? styles.flex2 : {}]}
+                        >
+                            <TextInput
+                                disableKeyboard
+                                onLayout={(e) => {
+                                    setContainerInputWidth(e.nativeEvent.layout.width);
+                                }}
+                                ref={ref}
+                                inputID={inputID}
+                                forceActiveLabel
+                                icon={Expensicons.Calendar}
+                                label={label}
+                                accessibilityLabel={label}
+                                role={CONST.ROLE.PRESENTATION}
+                                value={selectedDate}
+                                placeholder={placeholder ?? translate('common.dateFormat')}
+                                errorText={errorText}
+                                containerStyles={containerStyles}
+                                disabled={disabled}
+                                onFocus={() => show?.current?.()}
+                                onBlur={() => hideTooltip()}
+                            />
+                        </View>
+                    );
+                }}
+            </GenericTooltip>
+        </View>
+    );
}

Explains: I am using GenericTooltip to render the date picker. it will wrap the text input component. The similar is used here:

  1. Then, in CalendarPicker, pass onMouseDown={(e) => e.preventDefault()} for each PressableWithFeedback such as:



    and
    <PressableWithoutFeedback

It will make sure the input is not blurred when we click on any button in the date picker.

  1. Update the selected state of the date to match our designs (green background, bold text, with the same text color we use for our green buttons). => This part is very easy, can be addressed in PR phase.

  2. Auto focus on input: Just need to useuseAutoFocusInput in where we want to focus. For example:

                    <InputWrapper ref={inputCallbackRef} /> 
    const {inputCallbackRef} = useAutoFocusInput()

What alternative solutions did you explore? (Optional)

Result

Screen.Recording.2024-11-27.at.17.28.40.mov

@Pholenk
Copy link

Pholenk commented Nov 16, 2024

Edited by proposal-police: This proposal was edited at 2024-11-21 19:23:22 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Update Datepicker UX

What is the root cause of that problem?

UX enhancement

What changes do you think we should make in order to solve the problem?

  • Add this line to track whether the input is focused, popover relative position, and container height
    const [isFocused, setIsFocused] = useState(false);
    const [popoverPosition, setPopoverPosition] = useState({
        top: 0,
        left: 0,
        bottom: 0,
        right: 0,
    });
    const [containerSize, setContainerSize] = useState({
        height: 50,
    });

to this line

  • Add this line to set reference locally so the functionality of the popover and the text input is not dependent to the ref props
const localRef = useRef<BaseTextInputRef>(null);
  • Add these lines to set popover show or hide dynamically
    const popoverDisplayStyle = useMemo(() => {
        if (isFocused) {
            return styles.datePickerPopoverShow;
        }

        return styles.datePickerPopoverHide;
    }, [isFocused, styles.datePickerPopoverHide, styles.datePickerPopoverShow]);
  • Change the styles in here
+        datePickerRoot: {
-             position: 'relative',
-             zIndex: 99,
+            width: '100%',
+        },
+        datePickerPopoverShow: {
+            display: 'flex',
+        },
+
+        datePickerPopoverHide: {
+            display: 'none',
+        },
  • To expose the reference of this component add these lines
useImperativeHandle(
        ref,
        () => {
            if (!localRef.current) {
                return {} as BaseTextInputRef;
            }
            return localRef.current;
        },
        [],
    );
  • Add these lines to simplify the changes of isFocused state
const toggleFocus = useCallback(() => {
        setIsFocused((prevState) => !prevState);
    }, []);
  • Change the component code
-        <View style={styles.datePickerRoot}>
-            <View style={[shouldUseNarrowLayout ? styles.flex2 : {}, styles.pointerEventsNone]}>
+        <View style={[styles.flex0, styles.datePickerRoot, containerSize]}>
+            <View style={[shouldUseNarrowLayout ? styles.flex2 : {}]}>
                <TextInput
-                    ref={ref}
+                   ref={localRef}
                    inputID={inputID}
                    forceActiveLabel
                    icon={Expensicons.Calendar}
                    label={label}
                    accessibilityLabel={label}
                    role={CONST.ROLE.PRESENTATION}
                    value={selectedDate}
                    placeholder={placeholder ?? translate('common.dateFormat')}
                    errorText={errorText}
                    containerStyles={containerStyles}
                    textInputContainerStyles={[styles.borderColorFocus]}
-                    inputStyle={[styles.pointerEventsNone]}
                    disabled={disabled}
-                    readOnly
+                    readOnly={false}
+                    disableKeyboard
+                    onFocus={toggleFocus}
+                    onBlur={toggleFocus}
+                    onLayout={onLayoutHandle}
                />
            </View>
            <View
-                style={[styles.datePickerPopover, styles.border]}
+                style={[styles.datePickerPopover, styles.border, popoverDisplayStyle, popoverPosition]}
                collapsable={false}
            >
                <CalendarPicker
                    minDate={minDate}
                    maxDate={maxDate}
                    value={selectedDate}
                    onSelected={onSelected}
                />
            </View>
        </View>

Result

Mobile

screencast-Genymotion-2024-11-21_23.54.13.555.mp4

Web

simplescreenrecorder-2024-11-22_02.14.18.mp4

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@daledah
Copy link
Contributor

daledah commented Nov 16, 2024

Proposal updated

Copy link

melvin-bot bot commented Nov 19, 2024

@dannymcclain, @thesahindia Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Nov 19, 2024
@dannymcclain
Copy link
Contributor Author

@thesahindia are you the official proposal reviewer for this one?

@thesahindia
Copy link
Member

@thesahindia are you the official proposal reviewer for this one?

Yeah, I am assigned as the C+.

@melvin-bot melvin-bot bot removed the Overdue label Nov 19, 2024
@thesahindia
Copy link
Member

Thanks for the proposals everyone! Could you guys please share the end results?

@Pholenk
Copy link

Pholenk commented Nov 21, 2024

I've updated my proposal to add the video of the change result.

cc: @thesahindia

Copy link

melvin-bot bot commented Nov 22, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Nov 22, 2024
@dannymcclain
Copy link
Contributor Author

Not overdue. I think proposals are still being reviewed.

Copy link

melvin-bot bot commented Nov 25, 2024

@dannymcclain, @thesahindia Eep! 4 days overdue now. Issues have feelings too...

@thesahindia
Copy link
Member

Thanks for the proposals everyone! Could you guys please share the end results?

cc: @daledah @neonbhai

@perunt
Copy link
Contributor

perunt commented Jan 23, 2025

So, the question here is: how should the year selection page look? Should it remain the same as before?
Before:

RPReplay_Final1737650367.MP4

@shawnborton
Copy link
Contributor

The main issue I faced here is selecting a year. Previously, this was done by navigating to a separate screen. Now, it seems it should appear in a popup (on the web) or in a bottom sheet modal(mobile).

Can we see what it's like to navigate from the bottom-sheet date picker, to the RHP year selection? Maybe it won't feel so terrible?

@dannymcclain
Copy link
Contributor Author

Can we see what it's like to navigate from the bottom-sheet date picker, to the RHP year selection? Maybe it won't feel so terrible?

Agree it'd be nice to see. I think I'm digging the bottom sheet for the calendar, so if the RHP for year selection doesn't work, I wonder if we could just push to another view inside the bottom sheet/popover and do year selection like most other date pickers? Thinking something like this:

Image

@Expensify/design for thoughts on that!

I think it'd also be nice to adjust the styling of the calendar popover in the bottom sheet so we don't have that extra border/container inside the popover if possible.

Current Slight Adjustment
Image Image

@shawnborton
Copy link
Contributor

I definitely agree with your suggestion about the style adjustment of the calendar.

As for the year, definitely curious to see the RHP prototype first. I like what you have but it does feel a little less usable for people that just want to type in the year?

@dannymcclain
Copy link
Contributor Author

As for the year, definitely curious to see the RHP prototype first. I like what you have but it does feel a little less usable for people that just want to type in the year?

Totally agree—just wanted to offer it as an alternate.

@perunt
Copy link
Contributor

perunt commented Jan 24, 2025

Can we see what it's like to navigate from the bottom-sheet date picker, to the RHP year selection? Maybe it won't feel so terrible?

Currently, navigating from the bottom-sheet date picker to the year selection in RHP requires implementing modal screens through Navigation, not React Native (our current approach)
@chrispader is working on adding Modal Screens for Navigation. Once complete, we'll be able to navigate from BottomSheet/modal screens to anywhere in the app.
This technical constraint is why @dannymcclain 's design approach makes sense to me

@shawnborton
Copy link
Contributor

Hmm how long until that stuff is merged? I feel like we should try to reuse the RHP because it already exists and works great, we wouldn't need to build anything new here.

@perunt
Copy link
Contributor

perunt commented Jan 24, 2025

Hmm how long until that stuff is merged?

Work on implementing Navigation modal screens starts Monday (#53493)

As for the year, definitely curious to see the RHP prototype first.

Here's a rough proof-of-concept using modal-in-modal that we can use temporarily:

11.MP4

Quick demo of potential web transition (very rough, just to illustrate the concept):

Untitled.mov

@shawnborton
Copy link
Contributor

I don't mind that! On web, when you select a year, I would expect the datepicker to still be open when you get back to the original page.

@dannymcclain
Copy link
Contributor Author

I don't mind that!

Yeah it's really not bad! Feels way less weird than I was expecting.

On web, when you select a year, I would expect the datepicker to still be open when you get back to the original page.

I agree. Since this video is just a rough mock, I'm assuming/hoping we could make that happen in the actual version.

@perunt
Copy link
Contributor

perunt commented Jan 24, 2025

Yeah, it should stay open until we select a date or click outside the modal. Okay, then it looks like we'll stick with this implementation.

I'll use this design for the modal
Image

I'll keep you updated on how it's going

@shawnborton
Copy link
Contributor

Sounds great, I like where this is going - thanks @perunt !

@melvin-bot melvin-bot bot added the Overdue label Jan 27, 2025
@dannymcclain
Copy link
Contributor Author

Not overdue Melvin. @perunt is on it.

@melvin-bot melvin-bot bot removed the Overdue label Jan 27, 2025
@perunt
Copy link
Contributor

perunt commented Jan 29, 2025

A small update on this.

Untitled.mov
RPReplay_Final1738170552.MP4

@dannymcclain
Copy link
Contributor Author

cc @Expensify/design ☝

I think this is coming along nicely!

@shawnborton
Copy link
Contributor

Nice! I dig it.

@dubielzyk-expensify
Copy link
Contributor

Very exciting to see. Great job so far @perunt

@chrispader
Copy link
Contributor

Currently, navigating from the bottom-sheet date picker to the year selection in RHP requires implementing modal screens through Navigation, not React Native (our current approach) @chrispader is working on adding Modal Screens for Navigation. Once complete, we'll be able to navigate from BottomSheet/modal screens to anywhere in the app.

Hmm how long until that stuff is merged? I feel like we should try to reuse the RHP because it already exists and works great, we wouldn't need to build anything new here.

Sorry for the delay on the reply. The modal screen PR will need some more time, so i don't think waiting on the modal screen migration would be a good idea as of right now, but i see you already went that way anyway :)

@dannymcclain
Copy link
Contributor Author

@perunt what are your thoughts on the comment above? Are we blocked from implementing this in the way you showed in your video for now?

@perunt
Copy link
Contributor

perunt commented Feb 5, 2025

No, it's not a blocker at all. In fact, I've already implemented it

I noticed a reviewer’s comment suggesting that we should use this component everywhere. Should I update the other screens as well?

@shawnborton
Copy link
Contributor

I think that would be lovely.

@perunt
Copy link
Contributor

perunt commented Feb 7, 2025

There were around 10 places with the calendar component. I just replaced it and removed the old component. Testing throughout the app, and it looks good. Waiting for a review, as all the places need to be checked

@dannymcclain
Copy link
Contributor Author

@shubham1206agra @perunt What are the next steps here? How can we push this forward this week?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2
Projects
Status: HIGH
Development

No branches or pull requests