-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
chore: Datepicker Widget #36605
chore: Datepicker Widget #36605
Changes from all commits
2cf7b03
f856211
7f8c87a
333fd4d
321cc43
8f30f3a
d2cde29
733f523
9443462
63a5e91
2ce40e9
4daa071
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from "./src"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import React from "react"; | ||
import type { | ||
DateValue, | ||
CalendarProps as HeadlessCalendarProps, | ||
} from "react-aria-components"; | ||
import { | ||
CalendarGrid as HeadlessCalendarGrid, | ||
CalendarGridBody as HeadlessCalendarGridBody, | ||
CalendarGridHeader as HeadlessCalendarGridHeader, | ||
Calendar as HeadlessCalendar, | ||
} from "react-aria-components"; | ||
import { Flex, IconButton } from "@appsmith/wds"; | ||
|
||
import styles from "./styles.module.css"; | ||
import { CalendarCell } from "./CalendarCell"; | ||
import { CalendarHeading } from "./CalendarHeading"; | ||
import { CalendarHeaderCell } from "./CalendarHeaderCell"; | ||
|
||
type CalendarProps<T extends DateValue> = HeadlessCalendarProps<T>; | ||
|
||
export const Calendar = <T extends DateValue>(props: CalendarProps<T>) => { | ||
return ( | ||
<HeadlessCalendar {...props} className={styles.calendar}> | ||
<Flex alignItems="center" justifyContent="space-between" width="100%"> | ||
<IconButton icon="chevron-left" slot="previous" variant="ghost" /> | ||
<CalendarHeading size="subtitle" /> | ||
<IconButton icon="chevron-right" slot="next" variant="ghost" /> | ||
</Flex> | ||
<HeadlessCalendarGrid> | ||
<HeadlessCalendarGridHeader> | ||
{(day) => <CalendarHeaderCell>{day}</CalendarHeaderCell>} | ||
</HeadlessCalendarGridHeader> | ||
<HeadlessCalendarGridBody> | ||
{(date) => <CalendarCell date={date} />} | ||
</HeadlessCalendarGridBody> | ||
</HeadlessCalendarGrid> | ||
</HeadlessCalendar> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import React from "react"; | ||
import { Text } from "@appsmith/wds"; | ||
import { CalendarCell as HeadlessCalendarCell } from "react-aria-components"; | ||
|
||
import styles from "./styles.module.css"; | ||
import type { CalendarCellProps } from "./types"; | ||
|
||
function CalendarCell(props: CalendarCellProps) { | ||
const { date } = props; | ||
|
||
return ( | ||
<HeadlessCalendarCell {...props} className={styles["calendar-cell"]}> | ||
<Text>{date.day}</Text> | ||
</HeadlessCalendarCell> | ||
); | ||
} | ||
|
||
export { CalendarCell }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import React from "react"; | ||
import { Text } from "@appsmith/wds"; | ||
import { CalendarHeaderCell as HeadlessCalendarHeaderCell } from "react-aria-components"; | ||
|
||
import type { CalendarHeaderCellProps } from "./types"; | ||
|
||
function CalendarHeaderCell(props: CalendarHeaderCellProps) { | ||
const { children } = props; | ||
|
||
return ( | ||
<HeadlessCalendarHeaderCell {...props}> | ||
<Text color="neutral" fontWeight={700} textAlign="center"> | ||
{children} | ||
</Text> | ||
Comment on lines
+12
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Let's discuss your styling choices, shall we? I appreciate your use of the Consider making these style properties customizable through props. This way, the component can adapt to different contexts, just like how we adjust our teaching methods for different students. Here's an example: function CalendarHeaderCell({
children,
color = "neutral",
fontWeight = 700,
textAlign = "center",
...props
}: CalendarHeaderCellProps & TextProps) {
return (
<HeadlessCalendarHeaderCell {...props}>
<Text color={color} fontWeight={fontWeight} textAlign={textAlign}>
{children}
</Text>
</HeadlessCalendarHeaderCell>
);
} This approach allows users of your component to customize the styling if needed, while still providing sensible defaults. |
||
</HeadlessCalendarHeaderCell> | ||
); | ||
} | ||
|
||
export { CalendarHeaderCell }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import { Text, type TextProps } from "@appsmith/wds"; | ||
import React, { forwardRef, type ForwardedRef } from "react"; | ||
import { HeadingContext, useContextProps } from "react-aria-components"; | ||
|
||
function CalendarHeading( | ||
props: TextProps, | ||
ref: ForwardedRef<HTMLHeadingElement>, | ||
) { | ||
[props, ref] = useContextProps(props, ref, HeadingContext); | ||
const { children, ...domProps } = props; | ||
|
||
return ( | ||
<Text {...domProps} color="neutral" ref={ref}> | ||
{children} | ||
</Text> | ||
); | ||
} | ||
|
||
const _CalendarHeading = forwardRef(CalendarHeading); | ||
|
||
export { _CalendarHeading as CalendarHeading }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
export { Calendar } from "./Calendar"; | ||
export { CalendarCell } from "./CalendarCell"; | ||
export { CalendarHeading } from "./CalendarHeading"; | ||
export { CalendarHeaderCell } from "./CalendarHeaderCell"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
.calendar { | ||
padding: var(--outer-spacing-3); | ||
} | ||
|
||
.calendar table { | ||
display: flex; | ||
flex-direction: column; | ||
margin: 0; | ||
} | ||
|
||
.calendar thead tr { | ||
display: flex; | ||
justify-content: space-around; | ||
padding-block-start: var(--inner-spacing-1); | ||
} | ||
|
||
.calendar tbody tr { | ||
display: flex; | ||
justify-content: space-between; | ||
} | ||
|
||
.calendar thead th { | ||
display: flex; | ||
align-items: center; | ||
justify-content: center; | ||
inline-size: var(--sizing-9); | ||
block-size: var(--sizing-9); | ||
} | ||
|
||
.calendar tbody td { | ||
padding: var(--inner-spacing-1); | ||
} | ||
|
||
.calendar tbody [role="button"] { | ||
display: flex; | ||
align-items: center; | ||
justify-content: center; | ||
inline-size: var(--sizing-9); | ||
block-size: var(--sizing-9); | ||
border-radius: var(--border-radius-elevation-3); | ||
border: var(--border-width-2) solid transparent; | ||
text-align: center; | ||
} | ||
|
||
.calendar tbody [role="button"][data-disabled] { | ||
opacity: var(--opacity-disabled); | ||
} | ||
|
||
.calendar tbody [role="button"][data-hovered] { | ||
background-color: var(--color-bg-accent-subtle-hover); | ||
cursor: pointer; | ||
} | ||
|
||
.calendar tbody [role="button"][data-pressed] { | ||
background-color: var(--color-bg-accent-subtle-active); | ||
} | ||
|
||
.calendar tbody [role="button"][data-selected] { | ||
background-color: var(--color-bg-accent); | ||
color: var(--color-fg-on-accent); | ||
} | ||
|
||
.calendar tbody [role="button"][data-focus-visible], | ||
.calendar tbody [role="button"][data-focused] { | ||
border-color: var(--color-bd-accent); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { | ||
type CalendarCellProps as HeadlessCalendarCellProps, | ||
type CalendarHeaderCellProps as HeadlessCalendarHeaderCellProps, | ||
} from "react-aria-components"; | ||
|
||
export type CalendarCellProps = HeadlessCalendarCellProps & | ||
React.RefAttributes<HTMLTableCellElement>; | ||
|
||
export type CalendarHeaderCellProps = HeadlessCalendarHeaderCellProps & | ||
React.RefAttributes<HTMLTableCellElement>; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from "./src"; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,72 @@ | ||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||
FieldError, | ||||||||||||||||||||||||||||||
FieldDescription, | ||||||||||||||||||||||||||||||
FieldLabel, | ||||||||||||||||||||||||||||||
FieldCalenderPopover, | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct the spelling of 'Calendar' in 'FieldCalenderPopover' Attention to detail is important. There is a typo in your import statement. The component 'FieldCalenderPopover' should be spelled 'FieldCalendarPopover'. Let's correct this to ensure the component is properly imported and to avoid any runtime errors. Apply this diff to fix the typo: import {
FieldError,
FieldDescription,
FieldLabel,
- FieldCalenderPopover,
+ FieldCalendarPopover,
Button,
} from "@appsmith/wds"; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
Button, | ||||||||||||||||||||||||||||||
} from "@appsmith/wds"; | ||||||||||||||||||||||||||||||
import { getTypographyClassName } from "@appsmith/wds-theming"; | ||||||||||||||||||||||||||||||
import clsx from "clsx"; | ||||||||||||||||||||||||||||||
import React from "react"; | ||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||
DateInput, | ||||||||||||||||||||||||||||||
DateSegment, | ||||||||||||||||||||||||||||||
Group, | ||||||||||||||||||||||||||||||
DatePicker as HeadlessDatePicker, | ||||||||||||||||||||||||||||||
} from "react-aria-components"; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
import type { DatePickerProps } from "./types"; | ||||||||||||||||||||||||||||||
import styles from "../../ComboBox/src/styles.module.css"; | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just reusing the styels from combobox. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Today's lesson: Component Independence Now, let's talk about component design. Who can tell me what's unusual about this line? import styles from "../../ComboBox/src/styles.module.css"; That's correct, Sarah! We're importing styles from a different component. Can anyone tell me why this might not be the best practice? Excellent point, Michael! It creates a dependency between components that should ideally be independent. For your homework, I want you all to think about how we could create a separate styles file for our DatePicker. This way, if we ever need to change the ComboBox styles, we won't accidentally affect our DatePicker! Remember, class: in component design, independence is key! |
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
export const DatePicker = (props: DatePickerProps) => { | ||||||||||||||||||||||||||||||
const { | ||||||||||||||||||||||||||||||
contextualHelp, | ||||||||||||||||||||||||||||||
description, | ||||||||||||||||||||||||||||||
errorMessage, | ||||||||||||||||||||||||||||||
isDisabled, | ||||||||||||||||||||||||||||||
isLoading, | ||||||||||||||||||||||||||||||
isRequired, | ||||||||||||||||||||||||||||||
label, | ||||||||||||||||||||||||||||||
size = "medium", | ||||||||||||||||||||||||||||||
...rest | ||||||||||||||||||||||||||||||
} = props; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||
<HeadlessDatePicker | ||||||||||||||||||||||||||||||
aria-label={Boolean(label) ? undefined : "DatePicker"} | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pop quiz: Boolean logic! Alright, class, it's time for a pop quiz on Boolean logic! Can anyone spot the unnecessary operations in these lines? aria-label={Boolean(label) ? undefined : "DatePicker"}
color={Boolean(isLoading) ? "neutral" : "accent"}
variant={Boolean(isLoading) ? "ghost" : "filled"} Excellent observation, Emily! The - aria-label={Boolean(label) ? undefined : "DatePicker"}
+ aria-label={label ? undefined : "DatePicker"}
- color={Boolean(isLoading) ? "neutral" : "accent"}
+ color={isLoading ? "neutral" : "accent"}
- variant={Boolean(isLoading) ? "ghost" : "filled"}
+ variant={isLoading ? "ghost" : "filled"} Remember, class: simpler code is often clearer code. Always look for opportunities to remove redundant operations! Also applies to: 58-58, 62-62 🧰 Tools🪛 Biome
|
||||||||||||||||||||||||||||||
className={styles.formField} | ||||||||||||||||||||||||||||||
data-size={size} | ||||||||||||||||||||||||||||||
isDisabled={isDisabled} | ||||||||||||||||||||||||||||||
isRequired={isRequired} | ||||||||||||||||||||||||||||||
{...rest} | ||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||
{({ isInvalid }) => ( | ||||||||||||||||||||||||||||||
<> | ||||||||||||||||||||||||||||||
<FieldLabel | ||||||||||||||||||||||||||||||
contextualHelp={contextualHelp} | ||||||||||||||||||||||||||||||
isRequired={isRequired} | ||||||||||||||||||||||||||||||
text={label} | ||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||
<Group className={styles.inputWrapper}> | ||||||||||||||||||||||||||||||
<DateInput | ||||||||||||||||||||||||||||||
className={clsx(styles.input, getTypographyClassName("body"))} | ||||||||||||||||||||||||||||||
data-date-input | ||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||
{(segment) => <DateSegment segment={segment} />} | ||||||||||||||||||||||||||||||
</DateInput> | ||||||||||||||||||||||||||||||
<Button | ||||||||||||||||||||||||||||||
color={Boolean(isLoading) ? "neutral" : "accent"} | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplify the conditional by removing unnecessary Remember, simplicity is key in coding. The Apply this diff to simplify the code: color={Boolean(isLoading) ? "neutral" : "accent"}
+ color={isLoading ? "neutral" : "accent"} 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome
|
||||||||||||||||||||||||||||||
icon="calendar-month" | ||||||||||||||||||||||||||||||
isLoading={isLoading} | ||||||||||||||||||||||||||||||
size={size === "medium" ? "small" : "xSmall"} | ||||||||||||||||||||||||||||||
variant={Boolean(isLoading) ? "ghost" : "filled"} | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplify the conditional by removing unnecessary To enhance code clarity, we should avoid redundant type casting. In this case, Apply this diff to simplify the code: variant={Boolean(isLoading) ? "ghost" : "filled"}
+ variant={isLoading ? "ghost" : "filled"} 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome
|
||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||
Comment on lines
+57
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Time for a Boolean logic pop quiz! Alright, class, let's examine this Button component. Can anyone spot the redundant operations here? Yes, you in the back! Exactly right, we have two unnecessary <Button
- color={Boolean(isLoading) ? "neutral" : "accent"}
+ color={isLoading ? "neutral" : "accent"}
icon="calendar-month"
isLoading={isLoading}
size={size === "medium" ? "small" : "xSmall"}
- variant={Boolean(isLoading) ? "ghost" : "filled"}
+ variant={isLoading ? "ghost" : "filled"}
/> Excellent work! Remember, in conditional statements, variables are automatically coerced to booleans. We don't need to explicitly convert them. This makes our code cleaner and more efficient. Keep an eye out for these opportunities to simplify! 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome
|
||||||||||||||||||||||||||||||
</Group> | ||||||||||||||||||||||||||||||
<FieldError errorMessage={errorMessage} /> | ||||||||||||||||||||||||||||||
<FieldDescription description={description} isInvalid={isInvalid} /> | ||||||||||||||||||||||||||||||
<FieldCalenderPopover /> | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops! We've made the same spelling mistake twice! Class, do you remember the spelling error we corrected earlier in our import statement? Well, it looks like we've made the same mistake here in using the component. This is a great lesson in why we should use consistent naming throughout our code. Let's correct this line: - <FieldCalenderPopover />
+ <FieldCalendarPopover /> Now, can anyone tell me why it's important to be consistent with our naming? That's right! It helps prevent errors and makes our code easier to understand and maintain. Always double-check your spelling, especially when using custom components! 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
</> | ||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||
</HeadlessDatePicker> | ||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export { DatePicker } from "./DatePicker"; | ||
export type { DateValue } from "./types"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import type { | ||
DateValue, | ||
DatePickerProps as SpectrumDatePickerProps, | ||
ValidationResult, | ||
} from "react-aria-components"; | ||
import type { SIZES } from "@appsmith/wds"; | ||
|
||
export interface DatePickerProps | ||
extends Omit<SpectrumDatePickerProps<DateValue>, "slot" | "placeholder"> { | ||
/** The content to display as the label. */ | ||
label?: string; | ||
/** The content to display as the description. */ | ||
description?: string; | ||
/** The content to display as the error message. */ | ||
errorMessage?: string | ((validation: ValidationResult) => string); | ||
/** size of the select | ||
* | ||
* @default medium | ||
*/ | ||
size?: Omit<keyof typeof SIZES, "xSmall" | "large">; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Let's improve our 'size' property type, shall we? While your current implementation works, we can make it even better! Consider creating a separate type for the size property. This will make our code more readable and easier to maintain. Here's how we could refactor this: type DatePickerSize = Omit<keyof typeof SIZES, "xSmall" | "large">;
export interface DatePickerProps
extends Omit<SpectrumDatePickerProps<DateValue>, "slot" | "placeholder"> {
// ... other properties ...
size?: DatePickerSize;
// ... rest of the properties ...
} This way, if we need to use this size type elsewhere, we can easily reuse it! |
||
/** loading state for the input */ | ||
isLoading?: boolean; | ||
/** A ContextualHelp element to place next to the label. */ | ||
contextualHelp?: string; | ||
/** The content to display as the placeholder. */ | ||
placeholder?: string; | ||
} | ||
|
||
export type { DateValue }; |
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.
🛠️ Refactor suggestion
Let's think about making our component more flexible, shall we?
While rendering just the day works for basic cases, we might want to consider allowing customization of the cell content. What if we wanted to add events or highlight specific dates?
Consider modifying the component to accept a render prop or children, like this:
This way, users of your component can customize the cell content if needed, while still providing a default behavior.