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

[Bug]: Specifying defaultValue doesn't modify the timePicker #25730

Closed
2 tasks done
madhurauti opened this issue Nov 21, 2022 · 10 comments · Fixed by #26482
Closed
2 tasks done

[Bug]: Specifying defaultValue doesn't modify the timePicker #25730

madhurauti opened this issue Nov 21, 2022 · 10 comments · Fixed by #26482

Comments

@madhurauti
Copy link

Library

React / v8 (@fluentui/react)

System Info

System:
    OS: Windows 10 10.0.22621
    CPU: (8) x64 Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz
    Memory: 6.06 GB / 31.60 GB
  Browsers:
    Edge: Spartan (44.22621.819.0), Chromium (107.0.1418.42)
    Internet Explorer: 11.0.22621.1

Are you reporting Accessibility issue?

no

Reproduction

https://codepen.io/pen/?template=jOVZXaW

Bug Description

Browser: Chrome

image
The 6:00 AM Timepicker is incorrect. Based on the timepicker logic it should be set to 12:30 since the default value is set to 00:18 (see screenshot)

Actual Behavior

Specifying defaultValue for the timepicker doesn't set the value correctly.

Expected Behavior

Specifying defaultValue should reflect the value correctly in the timepicker.

Logs

NA

Requested priority

Blocking

Products/sites affected

No response

Are you willing to submit a PR to fix?

yes

Validations

  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • The provided reproduction is a minimal reproducible example of the bug.
@ling1726
Copy link
Member

@madhurauti please provide a code repro, issues without code repro will have less priority

@liu221127344
Copy link

The default value is not even working on the official website: https://developer.microsoft.com/en-us/fluentui#/controls/web/timepicker

@madhurauti
Copy link
Author

@ling1726 - I am not able to get the same behavior in sandbox. Hence, I cannot provide the sandbox link. In the examples shown here - https://developer.microsoft.com/en-us/fluentui#/controls/web/timepicker - all the timepicker controls start out empty. When timepicker is loaded with default value (for the first time it rounds to the next 30 minute interval - so if you pass 1:37 -> timepicker will have 2:00) and without default value (it rounds to the next 30 minute interval w.r.t current time). I believe this issue - #21824 also alludes to the problem I am facing.

if the timepicker cannot start with empty value then at least I want timepicker to take the default value and w.r.t that value render the next 30 minute interval, however, it seems this only happens when I load the timepicker for the first time.

Problem scenario (assume current time is 1:37 PM):

  1. Load timepicker (will load with 2:00 PM)
  2. Change timepicker to 3:00 PM.
  3. Reload the time picker with current time - 1:37 PM. Time picker default value is set to 1:37 PM but the value in the timepicker is still 3:00 PM - expectation is 1:37 PM should make it 2:00 PM.

@elvenprogrammer
Copy link

Same behavior, repro link: https://codepen.io/ElvenProgrammer/pen/oNMGErB?editors=1111

@bittola
Copy link

bittola commented Jan 19, 2023

the problem is that the userText is set to '' (an empty string) on initialization of the component. this is passed to combobox text prop and because of that the selection is cleared.

@elvenprogrammer
Copy link

Indeed, in my project I recreated the wrapper around the combobox and fixed it, but for sure it would be nice to have it supported natively (and v9 still doesn't support date nor time pickers)

@benvp
Copy link

benvp commented Feb 6, 2023

I ran into this issue today. It seems this PR (#24665) allowing comboBox to include an empty string has caused it. Starting from 8.99.0 - so for anybody stuck with this, a downgrade to 8.98.8 solved it for the moment.

@jsancho
Copy link

jsancho commented Feb 27, 2023

Indeed, in my project I recreated the wrapper around the combobox and fixed it, but for sure it would be nice to have it supported natively (and v9 still doesn't support date nor time pickers)

@elvenprogrammer could you add a little example on how have you managed to wrap the component to have it display a default time value?

I'd be ideal if you could update your sandbox, but a little guidance on what props to modify would already be fantastic :)

@elvenprogrammer
Copy link

Nothing fancy, I just copied and pasted the source code and slightly modified it:

import { useCallback, useMemo, useState } from "react";
import { ComboBox, IComboBoxOption, ITimeRange, TimePicker, ITimePickerProps, IComboBox, ITimePickerStrings } from "@fluentui/react";
import { useConst } from '@fluentui/react-hooks';
import {
    TimeConstants,
    addMinutes,
    formatTimeString,
    ceilMinuteToIncrement,
    getDateFromTimeSelection,
} from '@fluentui/date-time-utilities';

const REGEX_SHOW_SECONDS_HOUR_12 = /^((1[0-2]|0?[1-9]):([0-5][0-9]):([0-5][0-9])\s([AaPp][Mm]))$/;
const REGEX_HIDE_SECONDS_HOUR_12 = /^((1[0-2]|0?[1-9]):[0-5][0-9]\s([AaPp][Mm]))$/;
const REGEX_SHOW_SECONDS_HOUR_24 = /^([0-1]?[0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]$/;
const REGEX_HIDE_SECONDS_HOUR_24 = /^([0-1]?[0-9]|2[0-3]):[0-5][0-9]$/;

const TIME_LOWER_BOUND = 0;
const TIME_UPPER_BOUND = 23;

const getDefaultStrings = (useHour12: boolean, showSeconds: boolean): ITimePickerStrings => {
    const hourUnits = useHour12 ? '12-hour' : '24-hour';
    const timeFormat = `hh:mm${showSeconds ? ':ss' : ''}${useHour12 ? ' AP' : ''}`;
    const errorMessageToDisplay = `Enter a valid time in the ${hourUnits} format: ${timeFormat}`;

    return {
        invalidInputErrorMessage: errorMessageToDisplay,
    };
};

interface ITimeFieldProps {
    value: Date | undefined,
    onChange?: (time: Date | null) => void;
    onFormatDate?: (date: Date | undefined) => string;
    onValidateUserInput?: (userInput: string) => string;
    label: string;
}

const TimeField = ({
    value,
    onChange,
    onFormatDate,
    onValidateUserInput,
    label
}: ITimeFieldProps) => {
    const clampTimeRange = (timeRange: ITimeRange): ITimeRange => {
        return {
            start: Math.min(Math.max(timeRange.start, TIME_LOWER_BOUND), TIME_UPPER_BOUND),
            end: Math.min(Math.max(timeRange.end, TIME_LOWER_BOUND), TIME_UPPER_BOUND),
        };
    };

    const generateBaseDate = (increments: number, timeRange: ITimeRange | undefined, baseDate: Date) => {
        if (timeRange) {
            const clampedTimeRange = clampTimeRange(timeRange);
            baseDate.setHours(clampedTimeRange.start);
        }

        return ceilMinuteToIncrement(baseDate, increments);
    };

    const getDropdownOptionsCount = (increments: number, timeRange: ITimeRange | undefined) => {
        let hoursInRange = TimeConstants.HoursInOneDay;
        if (timeRange) {
            const clampedTimeRange = clampTimeRange(timeRange);
            if (clampedTimeRange.start > clampedTimeRange.end) {
                hoursInRange = TimeConstants.HoursInOneDay - timeRange.start - timeRange.end;
            } else if (timeRange.end > timeRange.start) {
                hoursInRange = timeRange.end - timeRange.start;
            }
        }
        return Math.floor((TimeConstants.MinutesInOneHour * hoursInRange) / increments);
    };

    const increments = 30;
    const showSeconds = true;
    const allowFreeform = true;
    const useHour12 = true;
    const timeRange = undefined;
    const strings = getDefaultStrings(useHour12, showSeconds);
    const defaultValue = value;

    const [userText, setUserText] = useState<string>('');
    const [errorMessage, setErrorMessage] = useState<string>('');

    const optionsCount = getDropdownOptionsCount(increments, timeRange);

    const initialValue = useConst(defaultValue || new Date());
    const baseDate: Date = useMemo(() => generateBaseDate(increments, timeRange, initialValue), [
        increments,
        timeRange,
        initialValue,
    ]);

    const timePickerOptions: IComboBoxOption[] = useMemo(() => {
        const optionsList = Array(optionsCount);
        for (let i = 0; i < optionsCount; i++) {
            optionsList[i] = 0;
        }

        return optionsList.map((_, index) => {
            const option = addMinutes(baseDate, increments * index);
            option.setSeconds(0);
            const optionText = onFormatDate ? onFormatDate(option) : formatTimeString(option, showSeconds, useHour12);
            return {
                key: optionText,
                text: optionText,
            };
        });
    }, [baseDate, increments, optionsCount, showSeconds, onFormatDate, useHour12]);

    const [selectedKey, setSelectedKey] = useState<string | number | undefined>(timePickerOptions[0].key);

    const onInputChange = useCallback(
        (event: React.FormEvent<IComboBox>, option?: IComboBoxOption, index?: number, value?: string): void => {
            const validateUserInput = (userInput: string): string => {
                let errorMessageToDisplay = '';
                let regex: RegExp;
                if (useHour12) {
                    regex = showSeconds ? REGEX_SHOW_SECONDS_HOUR_12 : REGEX_HIDE_SECONDS_HOUR_12;
                } else {
                    regex = showSeconds ? REGEX_SHOW_SECONDS_HOUR_24 : REGEX_HIDE_SECONDS_HOUR_24;
                }
                if (!regex.test(userInput)) {
                    errorMessageToDisplay = strings.invalidInputErrorMessage;
                }
                return errorMessageToDisplay;
            };

            const key = option?.key;
            let updatedUserText = '';
            let errorMessageToDisplay = '';
            if (value) {
                if (allowFreeform && !option) {
                    if (!onFormatDate) {
                        // Validate only if user did not add onFormatDate
                        errorMessageToDisplay = validateUserInput(value);
                    } else {
                        // Use user provided validation if onFormatDate is provided
                        if (onValidateUserInput) {
                            errorMessageToDisplay = onValidateUserInput(value);
                        }
                    }
                }
                updatedUserText = value;
            } else if (option) {
                updatedUserText = option.text;
            }

            if (onChange && !errorMessageToDisplay) {
                const selectedTime = value || option?.text || '';
                const date = selectedTime != ''
                    ? getDateFromTimeSelection(useHour12, baseDate, selectedTime)
                    : null;
                //onChange(event, date);
                onChange(date);
            }

            setErrorMessage(errorMessageToDisplay);
            setUserText(updatedUserText);
            setSelectedKey(key);
        },
        [
            baseDate,
            allowFreeform,
            onChange,
            onFormatDate,
            onValidateUserInput,
            showSeconds,
            useHour12,
            strings.invalidInputErrorMessage,
        ],
    );

    const text = onFormatDate ? onFormatDate(value) : (value ? formatTimeString(value, showSeconds, useHour12) : '');

    return (
        <>
            {/*<TimePicker
                useHour12
                allowFreeform
                autoComplete="on"
                label={'time'}
                defaultValue={new Date()}
                useComboBoxAsMenuWidth
                showSeconds={true}
            />*/}
            <ComboBox
                label={label}
                allowFreeform
                options={timePickerOptions}
                useComboBoxAsMenuWidth
                text={text}
                onChange={onInputChange}
            />
        </>
    );
}

export default TimeField;

@github-project-automation github-project-automation bot moved this from Parking-Lot to Done in Fluent UI - Shield Priors Apr 25, 2023
behowell pushed a commit that referenced this issue Apr 25, 2023
…ates (#26482)

* Update prop names, make renamed currentDate prop controlled

* Added DatePicker and TimePicker combination example

* Rephrase example text

* Combine ComboBox imports

* Fixed ComboBox import and updated DateTimePicker example

* Moved examples into standalone files

* Refactors and use new dateAnchor prop

* Updated all examples

* This should resolve the rest

* This should resolve the outstanding issues...

* Updated example strings

* Updated examples

* More changes to the basic example

* Removed useComboBoxAsMenuWidth prop

* Update formatTimeString function to return 00 if hours is 24

* Addressed comments

* Renamed baseDate parameter to dateStartAnchor

* Added prop onGetErrorMessage to get validation result

* Set selectedTime to invalid or undefined

* Clamp value to updated dateAnchor

* Make docs look nice and readable

* Removed unneeded imports

* Updated clampedStartAnchor initialization value

* Use internalDateAnchor

* Use fallbackDateAnchor and update DateTimePicker example

* Revert to ITimePickerProps to extend original omitted IComboBoxProps

* Addressed comments

* API snapshot update

* Updated TimePicker tests and added test for new controlled component variant

* Added test for handling changed base date anchor

* Verify selected time changes on dateAnchor change

* Added yarn change files

* Resolve linting errors

* Resolved more linting errors

* Resolved linting import error by in-lining stack and styles

* Addressed comments

* Revert onChange prop types to avoid breaking changes and pass React.FormEvent<IComboBox> to setSelectedTime callback

* Added explicit undefined pass to setSelectedTime in cases the dateAnchor changes

* Updated examples and call onChange outside of useControllableValue to pass in proper event type

* Control snapping of TimePicker values on DatePicker anchor change

* Added tests for using defaultValue or value as date anchors

* Took snapping logic out, fixed invalid key bug, and shared getDateAnchor function

* Pass in placeholder since placeholder prop no longer has default value

* Reflect optional timeOutOfBoundsErrorMessage string

* Updated tests and snapshot

* Addressed comments

* Removed unnecessary string state variables

* Followed comment suggestion and replaced onGetErrorMessage with onValidationError prop

* Added onValidationError example

* Added onValidationError test case

* Export TimePickerErrorData

* Renamed onValidationError and TimePickerErrorData to onValidationResult and TimePickerValidationData

* Renamed example to reflect new callback prop

* Use new example and fix casing

* Use onValidationResult and only call when stored error message differs from latest error message

* Added test for verifying onValidateResult only gets called on error message changes

* Split big test into two smaller tests
marcosmoura added a commit to marcosmoura/fluentui that referenced this issue Apr 25, 2023
* master:
  applying package updates
  microsoft#25730: TimePicker Default Value Fix, Controllable Usage, Example Updates (microsoft#26482)
  docs(react-datepicker-compat): Add description to README.md (microsoft#27677)
  Updated to use single hook selector (microsoft#27491)
  fix: Make border around Avatar's badge transparent using a mask (microsoft#27527)
  Disable focus on non-interactive elements (microsoft#27580)
  feat(react-drawer): implement "prevent close on click outside" feature (microsoft#27551)
marcosmoura added a commit to marcosmoura/fluentui that referenced this issue Apr 25, 2023
* master:
  applying package updates
  microsoft#25730: TimePicker Default Value Fix, Controllable Usage, Example Updates (microsoft#26482)
  docs(react-datepicker-compat): Add description to README.md (microsoft#27677)
  Updated to use single hook selector (microsoft#27491)
  fix: Make border around Avatar's badge transparent using a mask (microsoft#27527)
  Disable focus on non-interactive elements (microsoft#27580)
  feat(react-drawer): implement "prevent close on click outside" feature (microsoft#27551)
marcosmoura added a commit to marcosmoura/fluentui that referenced this issue Apr 25, 2023
* feat/drawer-components: (81 commits)
  docs: remove TODO marks
  fix: remove conflict code leftover
  applying package updates
  microsoft#25730: TimePicker Default Value Fix, Controllable Usage, Example Updates (microsoft#26482)
  docs(react-datepicker-compat): Add description to README.md (microsoft#27677)
  Updated to use single hook selector (microsoft#27491)
  fix: Make border around Avatar's badge transparent using a mask (microsoft#27527)
  Disable focus on non-interactive elements (microsoft#27580)
  feat(react-drawer): implement "prevent close on click outside" feature (microsoft#27551)
  fix(react-datepicker-compat): Make onValidationError onValidationResult so the error is updated when there's no longer an error (microsoft#27655)
  Fix `@fluentui/react-portal-compat`: to be compatible with React 18 (microsoft#27577)
  chore: fix versions of @fluentui/react-datepicker-compat (microsoft#27666)
  applying package updates
  applying package updates
  Make line chart screen reader accessible (microsoft#27632)
  fix(react-examples): Improve keyboard navigation in ContextualMenu.CustomMenuList (microsoft#25172)
  docs(react-textarea): Update examples to use Field (microsoft#27644)
  bugfix(react-dialog): `DialogTitle` root as `h2` by default (microsoft#27555)
  applying package updates
  chore: restructure stories, add separate category for flat tree (microsoft#27586)
  ...
@elvenprogrammer
Copy link

I tested the new version, now I'm able to control the selected value, but stil I'm not able to reset it, e.g. setting it to undefined:

<TimePicker placeholder="Select a time" showSeconds allowFreeform increments={30} autoComplete="on" label="Label" dateAnchor={new Date()} value={timeValue ? new Date(timeValue) : undefined} onChange={handleTimeChange} />

@microsoft microsoft locked as resolved and limited conversation to collaborators Jun 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging a pull request may close this issue.

9 participants