-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[datetime2] feat: DateRangeInput3 using react-day-picker v8 #6390
Conversation
fix lintBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
fix formattingBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
improve class naming patternBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self-review
|
||
import type { DateInput3Props } from "./dateInput3Props"; | ||
|
||
export function getFormattedDateString( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was unnecessarily copied from @blueprintjs/datetime; we now use it directly in @blueprintjs/datetime2 via the DatePickerUtils
import
/** Props shared between DateRangeInput v1 and v3 */ | ||
type DateRangeInputSharedProps = Omit<DateRangeInputProps, "dayPickerProps" | "locale" | "localeUtils" | "modifiers">; | ||
|
||
export type DateRangeInput3Props = DateRangeInputSharedProps & Pick<DateRangePicker3Props, "dayPickerProps" | "locale">; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file is quite similar to dateInput3Props.ts
, we might consider DRYing the two type implementations at some point (not in this PR though)
placement="bottom-start" | ||
{...popoverProps} | ||
autoFocus={false} | ||
className={classNames(Classes.DATE_RANGE_INPUT, popoverProps.className, this.props.className)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider using { Classes as DatetimeClasses } from "@blueprintjs/datetime"
import here, to make it more clear that this is a datetime "v1" class name
className={classNames(Classes.DATE_RANGE_INPUT, popoverProps.className, this.props.className)} | |
className={classNames(DatetimeClasses.DATE_RANGE_INPUT, popoverProps.className, this.props.className)} |
Related to #5652, #5699 - necessary to unblock React 18 support
Checklist
Changes proposed in this pull request:
Add new "v3" variant of DateRangeInput which uses react-day-picker v8 via DateRangePicker3 (#6375)
Reviewers should focus on:
Screenshot
Screen.Recording.2023-09-18.at.11.47.53.PM.mov