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

[@mantine/dates] Fixed : DateTimePicker not respecting minDate for the TimeInput #4890

Closed
wants to merge 4 commits into from

Conversation

rajeevdodda
Copy link

Fixes this issue #4596

@rtivital
Copy link
Member

It does not fix the issue in DateTimePicker, min and max time must be set based on minDate and maxDate

@rajeevdodda
Copy link
Author

@rtivital can you review my latest changes

@@ -35,17 +40,38 @@ export const TimeInput = factory<TimeInputFactory>((_props, ref) => {
props,
});

const validTime = () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function must be outside of component body, put it in the separate file

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created new file for this function

@@ -35,17 +40,38 @@ export const TimeInput = factory<TimeInputFactory>((_props, ref) => {
props,
});

const validTime = () => {
const minTime = dayjs(`2000-01-01 ${props.minTime}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do note reference props.minTime, props must be destructured on line 35.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed props reference

import classes from './TimeInput.module.css';

export interface TimeInputProps extends InputBaseProps, ElementProps<'input', 'size'> {
/** Determines whether seconds input should be rendered */
withSeconds?: boolean;
/** Minimum possible time */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There must be a new line between previous prop and new prop description, see other components for reference.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New lines inserted between props

@@ -10,11 +10,16 @@ import {
__InputStylesNames,
ElementProps,
} from '@mantine/core';
import dayjs from 'dayjs';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dayjs import must be before React import

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

const minTime = dayjs(`2000-01-01 ${props.minTime}`);
const maxTime = dayjs(`2000-01-01 ${props.maxTime}`);
const value = dayjs(`2000-01-01 ${props.value}`);
let _value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use let, return value if the condition matched instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated as per comment

type="time"
__staticSelector="TimeInput"
/>
<>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needless fragment

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@rtivital
Copy link
Member

rtivital commented Oct 1, 2023

It does not work, it is impossible to change TimeInput value in DateTimePicker

Story:

export function WithTimeMinMaxDate() {
  return (
    <div style={{ padding: 40, maxWidth: 400 }}>
      <DateTimePicker
        placeholder="Date time picker"
        defaultValue={new Date(2022, 3, 11)}
        clearable
        minDate={new Date(2022, 3, 10, 10, 50)}
        maxDate={new Date(2022, 3, 12, 10, 50)}
      />
    </div>
  );
}
2023-10-01.15.10.00.mov

@rtivital rtivital added the Invalid Incorrect issue or PR label Oct 1, 2023
@rtivital rtivital closed this Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid Incorrect issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants