Skip to content

chore: add datepicker component#37563

Merged
jsartisan merged 8 commits intoreleasefrom
chore/wds-datepicker
Nov 22, 2024
Merged

chore: add datepicker component#37563
jsartisan merged 8 commits intoreleasefrom
chore/wds-datepicker

Conversation

@jsartisan
Copy link
Contributor

@jsartisan jsartisan commented Nov 19, 2024

Added daterpicker component along with other components needed for it like Calendar andTimeField

Datepicker
CleanShot 2024-11-21 at 12 38 20@2x

Calendar
CleanShot 2024-11-21 at 12 38 54@2x

Timefield
CleanShot 2024-11-21 at 12 41 56@2x

/ok-to-test tags="@tag.Anvil"

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a Calendar component for date selection and display.
    • Added a DatePicker component for selecting dates and times with enhanced error handling.
    • Launched a TimeField component for time input with optional prefix and suffix.
    • Updated TextField component replacing the previous TextInput for improved usability.
  • Bug Fixes

    • Enhanced styling and responsiveness of input components.
  • Documentation

    • Added Storybook stories for Calendar, DatePicker, and TimeField components to showcase functionalities and configurations.
  • Chores

    • Refactored imports to utilize the new TextField component across various widgets.

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11970103158
Commit: a1a552c
Cypress dashboard.
Tags: @tag.Anvil
Spec:


Fri, 22 Nov 2024 10:01:23 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2024

Walkthrough

A new set of components has been introduced in the design system, including Calendar, CalendarCell, CalendarHeaderCell, CalendarHeading, DatePicker, and TimeField. These components are designed to enhance date and time selection functionalities within the application. Alongside these, CSS styles and Storybook stories have been added to support their usage and visualization. The TextInput component has been renamed to TextField, updating various references throughout the codebase.

Changes

File Change Summary
app/client/packages/design-system/widgets/src/components/Calendar/src/Calendar.tsx Added Calendar component and associated props type.
app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarCell.tsx Introduced CalendarCell component with props type.
app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeaderCell.tsx Added CalendarHeaderCell component and props type.
app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeading.tsx Introduced CalendarHeading component.
app/client/packages/design-system/widgets/src/components/Calendar/src/index.ts Exported Calendar, CalendarCell, CalendarHeading, and CalendarHeaderCell.
app/client/packages/design-system/widgets/src/components/Calendar/src/styles.module.css Added styles for the calendar component.
app/client/packages/design-system/widgets/src/components/Calendar/stories/Calendar.stories.tsx Created Storybook stories for Calendar with various configurations.
app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx Introduced DatePicker component with comprehensive date selection features.
app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx Added DatepickerTrigger component for user interaction.
app/client/packages/design-system/widgets/src/components/Datepicker/src/index.ts Exported DatePicker, types, and styles.
app/client/packages/design-system/widgets/src/components/Datepicker/src/styles.module.css Added styles for the Datepicker component.
app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts Defined types for DatePicker component.
app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx Created Storybook stories for DatePicker.
app/client/packages/design-system/widgets/src/components/TextField/src/TextField.tsx Renamed TextInput to TextField and updated type references.
app/client/packages/design-system/widgets/src/components/TextField/src/index.ts Exported TextField and its props type.
app/client/packages/design-system/widgets/src/components/TextField/src/types.ts Renamed TextInputProps to TextFieldProps.
app/client/packages/design-system/widgets/src/components/TextField/stories/TextField.stories.tsx Updated Storybook stories to reflect the TextField component.
app/client/packages/design-system/widgets/src/index.ts Updated exports to include new components and rename TextInput to TextField.
app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/component/index.tsx Updated import from TextInput to TextField.
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/component/index.tsx Updated import from TextInput to TextField.
app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget/component/index.tsx Updated import from TextInput to TextField.
app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/PageNumberInput.tsx Updated import from TextInput to TextField.
app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/Search.tsx Updated import from TextInput to TextField.
app/client/packages/design-system/widgets/src/components/Popover/src/Popover.tsx Modified Popover to accept a className prop.
app/client/packages/design-system/widgets/src/testing/ComplexForm.tsx Updated instances of TextInput to TextField.

Possibly related PRs

Suggested labels

ok-to-test

Suggested reviewers

  • ApekshaBhosale
  • sagar-qa007
  • yatinappsmith

🎉 In the realm of code, new components arise,
A calendar, a datepicker, a time field that flies!
With styles and stories, they come into view,
Renamed TextField, now fresh and anew!
So let’s celebrate this code, oh what a delight,
For in this design system, everything feels right! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Nov 19, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Outside diff range and nitpick comments (27)
app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx (1)

52-52: Remove redundant Boolean conversion

The Boolean call is unnecessary here. The ternary operator will coerce label to a boolean automatically.

Apply this diff to simplify the code:

- aria-label={Boolean(label) ? undefined : "DatePicker"}
+ aria-label={label ? undefined : "DatePicker"}
🧰 Tools
🪛 Biome

[error] 52-52: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

app/client/packages/design-system/widgets/src/components/Calendar/src/types.ts (1)

6-10: LGTM! Consider adding JSDoc comments.

The type definitions are well-structured and correctly extend the headless components with ref attributes.

Add JSDoc comments to improve documentation:

+/**
+ * Props for calendar cell component extending headless calendar cell props with ref attributes.
+ */
export type CalendarCellProps = HeadlessCalendarCellProps &
  React.RefAttributes<HTMLTableCellElement>;

+/**
+ * Props for calendar header cell component extending headless calendar header cell props with ref attributes.
+ */
export type CalendarHeaderCellProps = HeadlessCalendarHeaderCellProps &
  React.RefAttributes<HTMLTableCellElement>;
app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts (1)

8-14: Consider adding JSDoc comments for better documentation.

The interface structure is well-defined with appropriate type constraints and extensions.

Add JSDoc comments to improve documentation:

+/**
+ * Props for the TimeField component
+ * @template T - Type that extends TimeValue
+ */
export interface TimeFieldProps<T extends TimeValue>
  extends AriaTimeFieldProps<T>,
    FieldProps {
+  /** Optional suffix element to be rendered after the input */
  suffix?: ReactNode;
+  /** Optional prefix element to be rendered before the input */
  prefix?: ReactNode;
+  /** Size variant for the field, excluding xSmall */
  size?: Omit<keyof typeof SIZES, "xSmall">;
}
app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts (2)

7-15: Add interface documentation

Consider adding JSDoc documentation for the DatePickerProps interface to describe its purpose and usage.

+/**
+ * Props for the DatePicker component
+ * @template T - Type extending DateValue
+ */
 export interface DatePickerProps<T extends DateValue>

14-14: Consider enforcing the default size value in the type

The JSDoc indicates a default value of 'medium', but this isn't enforced in the type system.

-  size?: Omit<keyof typeof SIZES, "xSmall" | "large">;
+  size?: Omit<keyof typeof SIZES, "xSmall" | "large"> | undefined = "medium";
app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarCell.tsx (2)

8-10: Add prop validation for required properties.

Consider adding prop validation to ensure date is always provided and has the expected shape.

function CalendarCell(props: CalendarCellProps) {
+ if (!props.date) {
+   throw new Error('date prop is required for CalendarCell');
+ }
  const { date } = props;

12-14: Consider being explicit with prop forwarding.

Instead of spreading all props, consider explicitly passing only the required props to avoid exposing unwanted props to the DOM.

- <HeadlessCalendarCell {...props} className={styles["calendar-cell"]}>
+ <HeadlessCalendarCell 
+   date={date}
+   className={styles["calendar-cell"]}
+   selected={props.selected}
+   disabled={props.disabled}
+ >
app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeaderCell.tsx (1)

7-17: Consider memoization and prop type safety

While the implementation is clean, consider these improvements:

  1. Memoize the component to prevent unnecessary re-renders
  2. Be explicit about which props are spread to avoid passing unintended props

Consider this implementation:

-function CalendarHeaderCell(props: CalendarHeaderCellProps) {
+const CalendarHeaderCell = React.memo((props: CalendarHeaderCellProps) => {
   const { children, ...restProps } = props;
 
   return (
-    <HeadlessCalendarHeaderCell {...props}>
+    <HeadlessCalendarHeaderCell {...restProps}>
       <Text color="neutral" fontWeight={700} textAlign="center">
         {children}
       </Text>
     </HeadlessCalendarHeaderCell>
   );
-}
+});
app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeading.tsx (1)

5-17: Add JSDoc documentation and consider performance optimization

The component implementation is solid, but could benefit from:

  1. JSDoc documentation for props and component purpose
  2. Explicit accessibility role for the heading
  3. Memoization using React.memo for performance

Consider applying these improvements:

+/**
+ * CalendarHeading component renders a heading element within a calendar context.
+ * @param props - Text component props
+ * @param ref - Forwarded ref for the heading element
+ */
+const CalendarHeading = React.memo(
 function CalendarHeading(
   props: TextProps,
   ref: ForwardedRef<HTMLHeadingElement>,
 ) {
   [props, ref] = useContextProps(props, ref, HeadingContext);
   const { children, ...domProps } = props;
 
   return (
-    <Text {...domProps} color="neutral" ref={ref}>
+    <Text {...domProps} color="neutral" ref={ref} role="heading">
       {children}
     </Text>
   );
-}
+});
app/client/packages/design-system/widgets/src/components/TimeField/stories/TimeField.stories.tsx (2)

10-15: Consider enhancing component documentation

The current description is basic. Consider adding information about available props, usage examples, and accessibility features.

 parameters: {
   docs: {
     description: {
       component:
-        "A time input component that allows users to enter and select time values.",
+        "A time input component that allows users to enter and select time values. Supports 24-hour format, keyboard navigation, and screen readers. \n\n" +
+        "## Props\n" +
+        "- `value`: Time value (Time object)\n" +
+        "- `label`: Input label\n" +
+        "- `isDisabled`: Disables the input\n" +
+        "- `isRequired`: Makes the input required\n" +
+        "- `isInvalid`: Shows error state\n" +
+        "- `errorMessage`: Error message to display",
     },
   },
 },

22-52: Consider adding more comprehensive story coverage

The current stories cover basic states well, but consider adding stories for:

  • Time range validation (min/max time)
  • Custom time format
  • With placeholder text
  • With helper text

Example addition:

export const WithTimeConstraints: Story = {
  args: {
    label: "Office Hours",
    minTime: new Time(9, 0),
    maxTime: new Time(17, 0),
    value: new Time(10, 30),
    helperText: "Select time between 9 AM and 5 PM"
  },
};
app/client/packages/design-system/widgets/src/components/TextField/src/TextField.tsx (2)

Line range hint 26-55: Consider explicit handling of controlled/uncontrolled states

While the component looks well-structured, consider adding explicit handling for controlled vs uncontrolled input scenarios.

 export function TextField(props: TextFieldProps) {
   const {
     contextualHelp,
     errorMessage,
     isDisabled,
     isInvalid,
     isLoading,
     isReadOnly,
     isRequired,
     label,
     prefix,
     size = "medium",
     suffix,
     type,
     value,
+    defaultValue,
     ...rest
   } = props;

Update remaining TextInput references in ComplexForm.tsx

The component rename from TextInput to TextField is incomplete. Found references to the old name in:

  • app/client/packages/design-system/widgets/src/testing/ComplexForm.tsx

All other matches are either:

  • CSS class names and styles
  • Different components (NumberInput, Input)
  • Documentation references
  • Comments
🔗 Analysis chain

Line range hint 8-24: Verify component renaming across the codebase

The TextField rename looks good, but let's verify all TextInput references have been updated.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining TextInput references that might have been missed
rg -g '!*.{md,txt}' -i "textinput" ./app/client/packages/design-system/

# Search for new TextField usage to ensure consistent adoption
rg -g '!*.{md,txt}' -i "textfield" ./app/client/packages/design-system/

Length of output: 8042

app/client/packages/design-system/widgets/src/index.ts (1)

35-37: Consider grouping related components together

The date/time related components (Calendar, Datepicker, TimeField) are logically related but separated from other form components. Consider grouping them near other form-related components like TextField for better organization.

export * from "./components/TextField";
+export * from "./components/Calendar";
+export * from "./components/Datepicker";
+export * from "./components/TimeField";
export * from "./components/FieldLabel";
export * from "./components/Input";
export * from "./components/Field";
export * from "./components/Radio";
export * from "./components/ListBox";
export * from "./components/ListBoxItem";
export * from "./components/MenuItem";
export * from "./components/Markdown";
export * from "./components/Sidebar";
export * from "./components/Sheet";
-export * from "./components/Calendar";
-export * from "./components/Datepicker";
-export * from "./components/TimeField";
app/client/packages/design-system/widgets/src/components/Calendar/src/Calendar.tsx (2)

21-21: Add JSDoc documentation for the Calendar component.

Consider adding comprehensive documentation for the component props and usage examples.

+/**
+ * A calendar component that allows users to view and select dates.
+ * @template T - Type extending DateValue
+ * @param {CalendarProps<T>} props - Calendar component props
+ * @returns {JSX.Element} Calendar component
+ */
 export const Calendar = <T extends DateValue>(props: CalendarProps<T>) => {

21-39: Consider performance optimizations.

The component could benefit from memoization to prevent unnecessary re-renders, especially for the CalendarCell components.

+const MemoizedCalendarCell = React.memo(CalendarCell);
+
 export const Calendar = <T extends DateValue>(props: CalendarProps<T>) => {
   return (
     // ... other code ...
         <HeadlessCalendarGridBody>
-          {(date) => <CalendarCell date={date} />}
+          {(date) => <MemoizedCalendarCell date={date} />}
         </HeadlessCalendarGridBody>
app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx (2)

10-14: Add JSDoc comments to document the props interface.

Adding documentation will improve component usability and maintainability.

+/**
+ * Props for the DatepickerTrigger component
+ */
 interface DatepickerTriggerProps {
+  /** Whether the trigger is in loading state */
   isLoading?: boolean;
+  /** Size of the trigger button (small or medium) */
   size?: Omit<keyof typeof SIZES, "xSmall" | "large">;
+  /** Whether the trigger is disabled */
   isDisabled?: boolean;
 }

28-28: Simplify size prop handling.

Consider using a direct mapping for size values to avoid the conditional.

-        size={size === "medium" ? "small" : "xSmall"}
+        size={SIZE_MAPPING[size ?? "small"]}

Add this constant at the top of the file:

const SIZE_MAPPING = {
  small: "xSmall",
  medium: "small"
} as const;
app/client/packages/design-system/widgets/src/components/Calendar/src/styles.module.css (2)

1-3: Consider adding responsive constraints

Add min-width to prevent layout breaks on small screens.

 .calendar {
   padding: var(--outer-spacing-3);
+  min-width: 280px;
 }

34-66: Enhance interactive feedback

Consider these improvements for better user experience:

  1. Add transition for smoother state changes
  2. Add focus-ring styles for better keyboard navigation visibility
 .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;
+  transition: all 0.2s ease;
 }

 .calendar tbody [role="button"][data-focus-visible],
 .calendar tbody [role="button"][data-focused] {
   border-color: var(--color-bd-accent);
+  outline: 2px solid var(--color-bd-focus);
+  outline-offset: 2px;
 }
app/client/packages/design-system/widgets/src/components/Text/src/types.ts (1)

38-38: Add JSDoc documentation for optional children

Consider adding JSDoc documentation explaining valid use cases for omitting children, similar to other props in this interface.

-  /** The children of the component. */
+  /** The children of the component. If omitted, the component will render empty. 
+   * This is useful for components like CalendarHeading that may not always need content.
+   */
   children?: ReactNode;
app/client/packages/design-system/widgets/src/components/TimeField/src/TimeField.tsx (2)

67-69: Add accessibility attributes to prefix/suffix elements

The prefix and suffix spans should have appropriate aria-labels for better accessibility.

-        {Boolean(prefix) && <span data-input-prefix>{prefix}</span>}
-        {Boolean(suffix) && <span data-input-suffix>{suffix}</span>}
+        {prefix && <span aria-label="input prefix" data-input-prefix>{prefix}</span>}
+        {suffix && <span aria-label="input suffix" data-input-suffix>{suffix}</span>}

37-72: Consider adding error boundary

Since this is a form input component, consider wrapping it with an error boundary to gracefully handle rendering errors.

Would you like me to provide an example implementation of an error boundary for this component?

🧰 Tools
🪛 Biome

[error] 63-63: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

app/client/packages/design-system/widgets/src/components/TextField/stories/TextField.stories.tsx (2)

41-46: Consider extracting icon size to a constant

The icon size "medium" is repeated multiple times. Consider extracting it to a constant for better maintainability.

+ const ICON_SIZE = "medium" as const;
  <TextField
-   suffix={<Icon name="user" size="medium" />}
+   suffix={<Icon name="user" size={ICON_SIZE} />}
  />
  <TextField
-   prefix={<Icon name="user" size="medium" />}
+   prefix={<Icon name="user" size={ICON_SIZE} />}
  />

Line range hint 98-109: Enhance validation story with more robust validation

Consider these improvements:

  1. Add pattern validation for email
  2. Extract the error message to a constant
  3. Move the submit handler to a named function
+ const EMAIL_ERROR_MESSAGE = "Please enter a valid email address";
+ const handleSubmit = (e: React.FormEvent) => e.preventDefault();

  export const Validation: Story = {
    render: () => (
-     <Form onSubmit={(e) => e.preventDefault()}>
+     <Form onSubmit={handleSubmit}>
        <Flex direction="column" gap="spacing-3" width="sizing-60">
          <TextField
-           errorMessage="Please enter a valid email address"
+           errorMessage={EMAIL_ERROR_MESSAGE}
            isRequired
            label="Email"
            type="email"
+           pattern="[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$"
          />
app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx (2)

28-39: Consider using a dynamic default date

The hardcoded date '2023-06-15' might become outdated. Consider using a dynamic date relative to the current date.

-    value: parseDate("2023-06-15"),
+    value: parseDate(new Date().toISOString().split('T')[0]),

69-83: Enhance validation story with error state

The validation story only demonstrates the required field case. Consider adding examples with error states and different validation scenarios.

export const Validation: Story = {
  render: () => (
    <form
      onSubmit={(e) => {
        e.preventDefault();
        alert("Form submitted");
      }}
    >
      <Flex direction="column" gap="spacing-5" width="sizing-60">
        <DatePicker isRequired label="Required Field" />
+       <DatePicker 
+         label="With Error" 
+         errorMessage="Invalid date selected"
+         validationState="invalid"
+       />
        <Button type="submit">Submit</Button>
      </Flex>
    </form>
  ),
};
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 7612e4f and 12e25a3.

📒 Files selected for processing (30)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/Calendar.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarCell.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeaderCell.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeading.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/styles.module.css (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/stories/Calendar.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/styles.module.css (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css (2 hunks)
  • app/client/packages/design-system/widgets/src/components/Text/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/src/TextField.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/src/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/stories/TextField.stories.tsx (4 hunks)
  • app/client/packages/design-system/widgets/src/components/TextInput/src/index.ts (0 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/src/TimeField.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/src/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/stories/TimeField.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/index.ts (2 hunks)
  • app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/component/index.tsx (2 hunks)
💤 Files with no reviewable changes (1)
  • app/client/packages/design-system/widgets/src/components/TextInput/src/index.ts
✅ Files skipped from review due to trivial changes (9)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/index.ts
  • app/client/packages/design-system/widgets/src/components/Calendar/stories/Calendar.stories.tsx
  • app/client/packages/design-system/widgets/src/components/Datepicker/index.ts
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/index.ts
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/styles.module.css
  • app/client/packages/design-system/widgets/src/components/TextField/index.ts
  • app/client/packages/design-system/widgets/src/components/TextField/src/index.ts
  • app/client/packages/design-system/widgets/src/components/TimeField/index.ts
  • app/client/packages/design-system/widgets/src/components/TimeField/src/index.ts
🧰 Additional context used
🪛 Biome
app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx

[error] 52-52: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx

[error] 20-20: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)


[error] 24-24: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)


[error] 29-29: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

app/client/packages/design-system/widgets/src/components/TimeField/src/TimeField.tsx

[error] 63-63: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

🔇 Additional comments (30)
app/client/packages/design-system/widgets/src/components/TextField/src/types.ts (1)

Line range hint 5-10: LGTM! Interface rename follows design system conventions.

The interface structure is well-defined with appropriate type extensions and constraints.

Let's verify the rename impact:

app/client/packages/design-system/widgets/src/components/Calendar/src/types.ts (1)

1-10: Verify usage of these types in Calendar components.

Let's ensure these types are properly utilized in the related components.

✅ Verification successful

Types are properly utilized in Calendar components

The verification shows that:

  • CalendarCellProps is correctly imported and used in CalendarCell.tsx
  • CalendarHeaderCellProps is correctly imported and used in CalendarHeaderCell.tsx
  • Both components properly spread the props to their respective headless components
  • The type definitions extend the base types with the correct React ref attributes
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check usage of CalendarCellProps and CalendarHeaderCellProps in related components

# Search for usage of CalendarCellProps
echo "Checking CalendarCellProps usage:"
ast-grep --pattern 'CalendarCellProps'

# Search for usage of CalendarHeaderCellProps
echo "Checking CalendarHeaderCellProps usage:"
ast-grep --pattern 'CalendarHeaderCellProps'

# Look for any potential type errors in Calendar components
echo "Checking for type annotations in Calendar components:"
fd -e tsx -e ts --full-path '.*Calendar.*' -x cat {}

Length of output: 5756

app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts (1)

1-6: LGTM! Clean and well-organized imports.

The imports are properly structured and aliased where needed.

app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts (1)

1-17: LGTM! Clean and well-structured types

The type definitions are clean, well-organized, and properly leverage TypeScript's type system. The generic constraint and type composition are appropriately used.

app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarCell.tsx (2)

1-6: LGTM! Well-organized imports with good accessibility consideration.

The use of react-aria-components ensures proper accessibility support for the calendar cell.


18-18: LGTM! Appropriate export pattern.

Named export is suitable for a design system component.

app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeaderCell.tsx (3)

1-5: LGTM! Clean import structure

Imports are well-organized with clear separation between external and internal dependencies.


19-19: LGTM! Proper export syntax

Named export is appropriate for a design system component.


1-19: Verify integration with Calendar component

Let's ensure this component is properly integrated with its parent Calendar component.

app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeading.tsx (2)

1-3: LGTM! Clean and well-organized imports

All imports are properly typed and utilized in the implementation.


1-21: Verify Calendar component integration

Let's ensure the CalendarHeading is properly integrated within the Calendar component.

✅ Verification successful

CalendarHeading is properly integrated

The component is correctly imported and used within the Calendar component:

  • Imported in Calendar.tsx: import { CalendarHeading } from "./CalendarHeading"
  • Used in the Calendar's JSX: <CalendarHeading size="subtitle" />
  • Properly exported in index.ts for external usage
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for CalendarHeading usage in Calendar component
ast-grep --pattern 'CalendarHeading $$$' 

# Check if Calendar component exists and imports CalendarHeading
rg -l "import.*CalendarHeading" --type typescript

Length of output: 129


Script:

#!/bin/bash
# Search for CalendarHeading usage in Calendar component
rg -g "*.{ts,tsx}" "CalendarHeading"

# Check Calendar component imports and implementation
fd -e ts -e tsx "Calendar" --exec cat {}

Length of output: 5023

app/client/packages/design-system/widgets/src/components/TimeField/stories/TimeField.stories.tsx (1)

2-2: Verify TimeField component exports

Let's ensure the TimeField component is properly exported from the package.

✅ Verification successful

TimeField component is properly exported and implemented

The component is correctly exported from index.ts and its implementation exists at the expected location.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if TimeField is properly exported
ast-grep --pattern 'export { TimeField }' app/client/packages/design-system/widgets/src/components/TimeField/src/index.ts

# Verify component implementation exists
fd TimeField.tsx -p app/client/packages/design-system/widgets/src/components/TimeField/src/

Length of output: 425

app/client/packages/design-system/widgets/src/components/TextField/src/TextField.tsx (1)

Line range hint 1-6: LGTM! Import statements are well-organized

The type rename from TextInputProps to TextFieldProps maintains consistency with the component rename.

app/client/packages/design-system/widgets/src/index.ts (1)

24-24: Verify TextInput to TextField migration

The renaming from TextInput to TextField needs to be verified across the codebase to ensure all imports are updated.

✅ Verification successful

The previous search might have been too specific. Let's broaden the search to find any usage of these components, regardless of import path, and also locate the component files themselves to verify the migration.


The previous search revealed multiple TextField files, but we need to verify the actual component migration and its usage. Let's check the content of the new TextField component and search for any remaining TextInput references.


The previous search showed some TextInput usages in testing files and old components. Let's verify if these are related to the design system migration.


TextInput to TextField migration is safe to proceed

The search results confirm that:

  • No TextInput component exists in the design system packages
  • The only TextInput references are in admin settings test files, which are unrelated to the design system
  • The new TextField component is properly implemented with React Aria components
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining TextInput imports that need updating

# Search for any remaining TextInput imports
rg -l "import.*TextInput.*design-system"

# Search for new TextField imports to confirm migration
rg -l "import.*TextField.*design-system"

Length of output: 85


Script:

#!/bin/bash
# Let's try a broader search for TextInput and TextField usage
# First, find all potential source files
echo "=== Searching for TextInput usage ==="
rg -i "TextInput" --type ts --type tsx -A 2

echo -e "\n=== Searching for TextField usage ==="
rg -i "TextField" --type ts --type tsx -A 2

echo -e "\n=== Checking component definition ==="
fd -e ts -e tsx TextField

Length of output: 887


Script:

#!/bin/bash
# Let's try without file type restrictions and check actual content
echo "=== Searching for TextInput imports and usage ==="
rg "TextInput" -A 2

echo -e "\n=== Checking TextField component exports ==="
cat app/client/packages/design-system/widgets/src/components/TextField/src/TextField.tsx

echo -e "\n=== Checking TextField imports ==="
rg "TextField.*design-system" -A 2

Length of output: 37635


Script:

#!/bin/bash
# Let's check for any TextInput component definitions and usages in the design system
echo "=== Checking for TextInput component definitions ==="
fd -e tsx -e ts TextInput app/client/packages/design-system

echo -e "\n=== Checking for TextInput imports in design system ==="
rg -l "import.*TextInput.*" app/client/packages/design-system

# Let's also check the test files to ensure the migration is complete
echo -e "\n=== Checking test files for TextInput ==="
fd -e test.tsx -e test.ts . | xargs rg "TextInput"

Length of output: 924

app/client/packages/design-system/widgets/src/components/Calendar/src/Calendar.tsx (2)

1-19: LGTM! Clean imports and type definitions.

The imports are well-organized and the type definitions are properly structured. Good practice using the "Headless" prefix for react-aria-components to distinguish them from the wrapped components.


23-23: Verify styles implementation.

Ensure that the styles.module.css file properly implements the calendar class styles.

✅ Verification successful

Styles implementation is properly defined

The .calendar class and its nested elements are correctly implemented in styles.module.css with appropriate styling for the calendar component, including layout, spacing, and interactive states.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the calendar class is defined in the styles file
rg -l "\.calendar\s*{" "$(dirname "$(fd -t f "Calendar.tsx" | head -n1)")/styles.module.css"

Length of output: 336


Script:

#!/bin/bash
# Check if the styles file exists and show its content
fd -t f "styles.module.css" app/client/packages/design-system/widgets/src/components/Calendar/src/ -x cat {}

Length of output: 1536

app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx (2)

1-8: LGTM! Well-organized imports with clear separation of concerns.


34-48: LGTM! Well-structured render implementation with proper accessibility support.

The component correctly implements accessible date input using react-aria components.

app/client/packages/design-system/widgets/src/components/Calendar/src/styles.module.css (3)

22-28: LGTM!

Good use of CSS variables for consistent sizing and proper content alignment.


30-32: LGTM!

Clean separation between cell and button styling.


11-20: Verify alignment consistency

The header uses space-around while the body uses space-between, which might cause subtle misalignment between header and body cells.

app/client/packages/design-system/widgets/src/components/Text/src/types.ts (1)

38-38: Verify the necessity of making children optional

Making the children prop optional could lead to empty text elements. Please ensure there are valid use cases for this change, particularly in the context of CalendarHeading usage.

app/client/packages/design-system/widgets/src/components/TimeField/src/TimeField.tsx (2)

1-19: LGTM! Well-organized imports

The imports are properly structured with clear separation between styling utilities and functional components.


21-35: Verify TimeFieldProps interface implementation

The component's prop types look well-structured, but let's verify the complete interface implementation.

✅ Verification successful

Props implementation is correctly structured and complete

The TimeField component properly implements all required props from both AriaTimeFieldProps and FieldProps interfaces. The destructured props in the component match the interface definitions:

  • Base props from AriaTimeFieldProps (isDisabled, isReadOnly, isRequired, value)
  • Field-specific props (errorMessage, label, contextualHelp)
  • Custom props (prefix, suffix, size)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for TimeFieldProps interface definition
ast-grep --pattern 'interface TimeFieldProps {
  $$$
}'

# Verify if there are any required props that aren't being handled
rg -A 5 'TimeFieldProps.*=' ./app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts

Length of output: 169


Script:

#!/bin/bash
# Let's try a different approach to find the interface definition
# First, let's find the types file
fd "types.ts" --type f ./app/client/packages/design-system/widgets/src/components/TimeField

# Then search for TimeFieldProps in all TypeScript files in TimeField directory
rg "TimeFieldProps" ./app/client/packages/design-system/widgets/src/components/TimeField -A 5 --type typescript

# Also check for any type imports related to TimeFieldProps
rg "import.*TimeFieldProps" ./app/client/packages/design-system/widgets/src/components/TimeField --type typescript

Length of output: 472


Script:

#!/bin/bash
# Let's try without file type specification
rg "TimeFieldProps" ./app/client/packages/design-system/widgets/src/components/TimeField -A 5

# Check the content of the types file we found
cat ./app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts

Length of output: 2786


Script:

#!/bin/bash
# Let's check what FieldProps includes to ensure all required props are handled
rg "interface FieldProps" -A 10 ./app/client/packages/design-system/widgets/src

# Also check if there are any other usages of TimeField to verify prop consistency
rg "TimeField.*props" ./app/client/packages/design-system/widgets/src

Length of output: 1449

app/client/packages/design-system/widgets/src/components/TextField/stories/TextField.stories.tsx (1)

4-8: LGTM! Component rename consistently applied

The imports and meta configuration correctly reflect the transition from TextInput to TextField.

app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx (1)

1-17: LGTM! Well-structured meta configuration.

The imports and meta configuration follow Storybook best practices and the Component Story Format.

app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/component/index.tsx (2)

3-3: LGTM: Import statement updated correctly

The TextField import aligns with the component renaming across the codebase.


49-49: LGTM: ElementType assignment updated appropriately

The conditional assignment has been correctly updated to use TextField while maintaining the existing logic for TextArea.

Let's verify the TextField component is consistently used across the codebase:

✅ Verification successful

Let me verify if the TextInput references found are related to the migration from TextInput to TextField or if they are different components.


Let me check the actual TextField component usage in the WDSInputWidget.


The TextField migration in WDSInputWidget is correctly implemented

The TextInput references found in the codebase are either styled components or different components from the ones being migrated. The WDSInputWidget correctly imports and uses TextField from @appsmith/wds, maintaining consistency with the migration plan.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining TextInput references that might have been missed
# Expect: No occurrences of TextInput except in migration/changelog files

rg -g '!*.{md,json}' -g '!CHANGELOG*' -g '!**/migrations/*' 'TextInput'

Length of output: 13343


Script:

#!/bin/bash
# Check the imports and usage of TextField in the codebase
ast-grep --pattern 'import { $$$, TextField, $$$ } from "$_"'

# Also check the definition of TextInput in @appsmith/wds
rg -g '*.{ts,tsx}' '@appsmith/wds.*TextInput'

Length of output: 273


Script:

#!/bin/bash
# Check the imports in WDSInputWidget
rg -g '*.{ts,tsx}' -A 5 'import.*TextField' app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/

# Check where TextField is imported from @appsmith/wds
rg -g '*.{ts,tsx}' '@appsmith/wds.*TextField'

Length of output: 878

app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css (2)

131-133: LGTM! Clean selector optimization.

The use of :is() pseudo-class improves readability and maintainability compared to multiple :not() selectors.


207-207: LGTM! Enhanced focus handling.

The combined focus state handling using :is() properly supports both direct and nested focus scenarios, which is essential for compound components like DatePicker.

@jsartisan jsartisan marked this pull request as draft November 20, 2024 11:59
@jsartisan jsartisan force-pushed the chore/wds-datepicker branch from 5f6a1d2 to b9e266e Compare November 21, 2024 07:04
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename TextInput to TextField to avoid name confusion with TextInput and Input.

@jsartisan jsartisan marked this pull request as ready for review November 21, 2024 07:28
<CalendarHeading size="subtitle" />
<IconButton icon="chevron-right" slot="next" variant="ghost" />
</Flex>
<HeadlessCalendarGrid>
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems that we need more styles here.

Снимок экрана 2024-11-21 в 15 25 35

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the storybook styles that we have applied on td and tr. :/

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we override it here?

);
}

const _CalendarHeading = forwardRef(CalendarHeading);
Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) You can simplify the export a bit here if you do the same as in our other files.

Copy link
Contributor Author

@jsartisan jsartisan Nov 22, 2024

Choose a reason for hiding this comment

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

I don't get it. Did you mean to use like:

const CalendarHeading = forwardRef((
  props: TextProps,
  ref: ForwardedRef<HTMLHeadingElement>,
) => {
  [props, ref] = useContextProps(props, ref, HeadingContext);
  const { children, ...domProps } = props;

  return (
    <Text {...domProps} color="neutral" ref={ref}>
      {children}
    </Text>
  );
});

export { CalendarHeading };

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean like we usually do.

const _Checkbox = (
  props: CheckboxProps,
  ref: ForwardedRef<HTMLLabelElement>,
) => {}; 

export const Checkbox = forwardRef(_Checkbox);

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Outside diff range and nitpick comments (19)
app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarCell.tsx (1)

10-11: Add JSDoc documentation for the exported type.

Consider adding documentation to describe the purpose and usage of the CalendarCellProps type.

+/**
+ * Props for the CalendarCell component.
+ * Extends HeadlessCalendarCellProps from react-aria-components with React ref attributes.
+ */
 export type CalendarCellProps = HeadlessCalendarCellProps &
   React.RefAttributes<HTMLTableCellElement>;
app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeaderCell.tsx (2)

7-8: Consider adding JSDoc comment for type documentation.

The type extension is well-structured, but could benefit from documentation explaining its purpose and usage.

+/**
+ * Props for CalendarHeaderCell component.
+ * Extends HeadlessCalendarHeaderCellProps with React ref attributes.
+ */
 export type CalendarHeaderCellProps = HeadlessCalendarHeaderCellProps &
   React.RefAttributes<HTMLTableCellElement>;

10-20: Consider extracting style constants and controlling props spread.

Two suggestions for improvement:

  1. Move hardcoded style values to theme constants
  2. Be explicit about which props are spread to avoid unexpected prop leakage
+import { FONT_WEIGHTS, TEXT_COLORS } from "../constants";
+
 function CalendarHeaderCell(props: CalendarHeaderCellProps) {
-  const { children } = props;
+  const { children, ...restProps } = props;
 
   return (
-    <HeadlessCalendarHeaderCell {...props}>
-      <Text color="neutral" fontWeight={700} textAlign="center">
+    <HeadlessCalendarHeaderCell {...restProps}>
+      <Text 
+        color={TEXT_COLORS.neutral}
+        fontWeight={FONT_WEIGHTS.bold}
+        textAlign="center"
+      >
         {children}
       </Text>
     </HeadlessCalendarHeaderCell>
app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget/component/index.tsx (1)

Migration from TextInput to TextField is incomplete

The codebase still contains custom TextInput components that haven't been migrated to use TextField from @appsmith/wds:

  • app/client/src/components/designSystems/appsmith/TextInputComponent.tsx - Custom TextInput using Blueprint's InputGroup
  • app/client/src/pages/AdminSettings/FormGroup/TextInput.tsx - Using FormTextField from utils

These components should be reviewed for migration to maintain consistency with the new TextField component.

🔗 Analysis chain

Line range hint 3-35: Verify complete migration of TextInput to TextField

Let's ensure no TextInput references remain in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining TextInput references that might have been missed

# Search for any remaining TextInput imports
echo "Checking for remaining TextInput imports..."
rg "import.*TextInput.*from.*@appsmith/wds"

# Search for any remaining TextInput usage
echo "Checking for remaining TextInput component usage..."
rg "<TextInput"

Length of output: 1088


Script:

#!/bin/bash
# Let's verify if these TextInput references are from @appsmith/wds or other sources
# and check the actual component definitions

# Check TextInputWrapper definition
echo "Checking TextInputWrapper definition..."
rg -A 5 "const TextInputWrapper"

# Check TextInput component in AdminSettings
echo "Checking AdminSettings TextInput component..."
cat app/client/src/pages/AdminSettings/FormGroup/TextInput.test.tsx

# Check if there are any imports of TextInput from @appsmith/wds specifically
echo "Checking TextInput imports from @appsmith/wds..."
rg "from.*@appsmith/wds.*TextInput"
rg "TextInput.*from.*@appsmith/wds"

Length of output: 3199


Script:

#!/bin/bash
# Let's check the actual TextInput component in AdminSettings and its imports
echo "Checking AdminSettings TextInput component source..."
cat app/client/src/pages/AdminSettings/FormGroup/TextInput.tsx

# Check the BaseTextInput component
echo "Checking BaseTextInput component..."
cat app/client/src/components/designSystems/appsmith/TextInputComponent.tsx

# Check if TextField is being used in these components
echo "Checking TextField usage in these files..."
rg "TextField" app/client/src/pages/AdminSettings/FormGroup/TextInput.tsx app/client/src/components/designSystems/appsmith/TextInputComponent.tsx

Length of output: 5904

app/client/packages/design-system/widgets/src/components/Calendar/stories/Calendar.stories.tsx (2)

8-14: Enhance component documentation

Consider expanding the documentation to include:

  • Available props and their descriptions
  • Usage examples
  • Accessibility features
 parameters: {
   docs: {
     description: {
-      component: "A calendar component for date selection and display.",
+      component: "A calendar component for date selection and display. Supports min/max date constraints, disabled and readonly states. Implements ARIA attributes for accessibility.",
     },
   },
 },

20-52: Add stories for error states and validation

The current stories cover basic use cases well. Consider adding:

  • Error state (invalid date)
  • Validation feedback
  • Custom date formatting
  • Different locale examples

Example story to add:

export const WithError: Story = {
  args: {
    defaultValue: today(getLocalTimeZone()),
    errorMessage: "Selected date is invalid",
    validationState: "invalid",
  },
};

export const CustomLocale: Story = {
  args: {
    defaultValue: today(getLocalTimeZone()),
    locale: "fr-FR",
  },
};
app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/component/index.tsx (1)

Line range hint 27-43: LGTM! Consider type enhancement for validation status

The TextField implementation correctly forwards all necessary props and handles validation states appropriately.

Consider adding a union type for validationStatus to restrict possible values:

type ValidationStatus = "valid" | "invalid" | undefined;
app/client/packages/design-system/widgets/src/components/Calendar/src/Calendar.tsx (2)

19-19: Add JSDoc documentation for the CalendarProps type.

Consider adding comprehensive documentation for the type definition to improve maintainability and developer experience.

+/**
+ * Props for the Calendar component.
+ * @template T - The type of date value used by the calendar
+ * @extends {HeadlessCalendarProps<T>}
+ */
type CalendarProps<T extends DateValue> = HeadlessCalendarProps<T>;

21-39: Consider adding error boundaries and prop validation.

The Calendar component could benefit from additional enterprise-level safeguards:

  1. Wrap with error boundary to handle rendering failures gracefully
  2. Add prop validation for required fields
  3. Consider memoization for performance optimization
+const CalendarErrorBoundary = withErrorBoundary(({ children }) => children);
+
 export const Calendar = <T extends DateValue>(props: CalendarProps<T>) => {
+  if (!props.aria-label) {
+    console.warn('Calendar: aria-label prop is recommended for accessibility');
+  }
+
   return (
+    <CalendarErrorBoundary>
     <HeadlessCalendar {...props} className={styles.calendar}>
       {/* ... existing implementation ... */}
     </HeadlessCalendar>
+    </CalendarErrorBoundary>
   );
-};
+}
app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx (1)

34-48: Add ARIA label for better accessibility.

Consider adding an aria-label to the Group component to improve screen reader experience.

-    <Group className={textInputStyles.inputGroup}>
+    <Group className={textInputStyles.inputGroup} aria-label="Date picker input">
app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/PageNumberInput.tsx (2)

Line range hint 49-65: Consider using proper event typing for onKeyDown

The implementation looks good, but the any type for the keyboard event could be more specific.

Consider updating the event type:

-      onKeyDown={(e: any) => {
+      onKeyDown={(e: React.KeyboardEvent<HTMLInputElement>) => {

Line range hint 57-58: Address TODO comment in current changes

Since we're already modifying this file, let's address the TODO comment now rather than leaving it for future changes.

The proper typing suggestion provided above will resolve both the TODO and allow removal of the eslint-disable comment.

app/client/packages/design-system/widgets/src/components/TimeField/src/TimeField.tsx (2)

21-35: Add JSDoc documentation for the TimeField component.

Consider adding JSDoc documentation to describe the component's purpose, generic type parameter, and props.

Example:

+/**
+ * A form field component for time input with optional prefix/suffix and validation.
+ * @template T - Type of time value, must extend TimeValue
+ */
export function TimeField<T extends TimeValue>(props: TimeFieldProps<T>) {

65-65: Consider memoizing the DateSegment renderer.

The segment renderer function could benefit from memoization to prevent unnecessary re-renders.

-          {(segment) => <DateSegment segment={segment} />}
+          {React.useCallback((segment) => <DateSegment segment={segment} />, [])}
app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx (3)

22-35: Consider adding a type definition for the size prop

The size prop defaulting to "medium" implies a finite set of possible values. Consider defining an enum or union type for better type safety.

type DatePickerSize = "small" | "medium" | "large";

36-46: Consider using type guards for safer type narrowing

The current type narrowing using the 'in' operator works but could be more explicit with custom type guards.

function isTimeValue(value: DateValue | null | undefined): value is TimeValue {
  return value != null && 'hour' in value;
}

60-67: Simplify time granularity check

The current implementation can be simplified using an array includes check.

- const timeGranularity =
-   state.granularity === "hour" ||
-   state.granularity === "minute" ||
-   state.granularity === "second"
-     ? state.granularity
-     : null;
+ const timeGranularity = ['hour', 'minute', 'second'].includes(state.granularity)
+   ? state.granularity
+   : null;
app/client/packages/design-system/widgets/src/testing/ComplexForm.tsx (2)

142-142: Add accessibility attributes to TextField

Even in test components, we should maintain accessibility standards.

-          <TextField />
+          <TextField aria-label="Test input" placeholder="Enter value" />

Line range hint 28-228: Consider enhancing test coverage with form management

Since this is a test component, consider integrating a form management library (e.g., React Hook Form) to test form validation scenarios and state management patterns that would be common in production use cases.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 12e25a3 and e067549.

📒 Files selected for processing (35)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/Calendar.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarCell.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeaderCell.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeading.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/styles.module.css (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Calendar/stories/Calendar.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/styles.module.css (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css (3 hunks)
  • app/client/packages/design-system/widgets/src/components/Popover/src/Popover.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Text/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/src/TextField.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/src/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TextField/stories/TextField.stories.tsx (4 hunks)
  • app/client/packages/design-system/widgets/src/components/TextInput/src/index.ts (0 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/src/TimeField.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/src/index.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/TimeField/stories/TimeField.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/index.ts (2 hunks)
  • app/client/packages/design-system/widgets/src/testing/ComplexForm.tsx (3 hunks)
  • app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/component/index.tsx (2 hunks)
  • app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/component/index.tsx (2 hunks)
  • app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget/component/index.tsx (2 hunks)
  • app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/PageNumberInput.tsx (2 hunks)
  • app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/Search.tsx (2 hunks)
💤 Files with no reviewable changes (1)
  • app/client/packages/design-system/widgets/src/components/TextInput/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (21)
  • app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeading.tsx
  • app/client/packages/design-system/widgets/src/components/Calendar/src/index.ts
  • app/client/packages/design-system/widgets/src/components/Calendar/src/styles.module.css
  • app/client/packages/design-system/widgets/src/components/Datepicker/index.ts
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/index.ts
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/styles.module.css
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts
  • app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx
  • app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css
  • app/client/packages/design-system/widgets/src/components/Text/src/types.ts
  • app/client/packages/design-system/widgets/src/components/TextField/index.ts
  • app/client/packages/design-system/widgets/src/components/TextField/src/TextField.tsx
  • app/client/packages/design-system/widgets/src/components/TextField/src/index.ts
  • app/client/packages/design-system/widgets/src/components/TextField/src/types.ts
  • app/client/packages/design-system/widgets/src/components/TextField/stories/TextField.stories.tsx
  • app/client/packages/design-system/widgets/src/components/TimeField/index.ts
  • app/client/packages/design-system/widgets/src/components/TimeField/src/index.ts
  • app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts
  • app/client/packages/design-system/widgets/src/components/TimeField/stories/TimeField.stories.tsx
  • app/client/packages/design-system/widgets/src/index.ts
  • app/client/src/modules/ui-builder/ui/wds/WDSInputWidget/component/index.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx

[error] 49-49: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx

[error] 20-20: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)


[error] 24-24: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)


[error] 29-29: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

app/client/packages/design-system/widgets/src/components/TimeField/src/TimeField.tsx

[error] 63-63: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

🔇 Additional comments (19)
app/client/packages/design-system/widgets/src/components/Popover/src/Popover.tsx (1)

9-9: LGTM! Clean implementation of className customization.

The className prop handling is implemented correctly using clsx, maintaining the base styles while allowing customization.

Let's verify the DatePicker usage of this component:

Also applies to: 12-12

✅ Verification successful

The className prop addition is safe and properly used across components

The verification shows that the Popover component is used in multiple components (DatePicker, Select, Modal, Menu, ComboBox) and the DatePicker correctly uses the className prop with datePickerStyles.popover. Other components also use the Popover with proper className handling, making this change backward compatible and well-integrated.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Popover usage in DatePicker component
ast-grep --pattern 'import { Popover } from "$$$"
$$$
<Popover $$$>
  $$$
</Popover>'

Length of output: 87


Script:

#!/bin/bash
# Let's try a broader search for Popover usage
rg -l "Popover" app/client/packages/design-system/widgets/src

# Then search for DatePicker files
fd "DatePicker" app/client/packages/design-system/widgets/src

# Look for actual Popover usage in components
rg -A 2 "<Popover" app/client/packages/design-system/widgets/src

Length of output: 3435

app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/Search.tsx (2)

2-2: LGTM: Import change aligns with the component migration.


Line range hint 15-21: Verify TextField props compatibility.

The component change looks good, but let's verify that all TextField props match the previous TextInput implementation to ensure no functionality is lost.

✅ Verification successful

Let me try a different search pattern to find the relevant component usages.


Let me try one more search with a broader pattern to find the component implementations.


Let me check the previous implementation to compare the props.


Let me check the TextInput component implementation and its props.


TextField props are compatible with the search functionality requirements

The migration from TextInput to TextField maintains the essential props needed for the search functionality:

  • onChange for handling search input
  • value for controlled input state
  • placeholder for user guidance
  • excludeFromTabOrder for accessibility control
  • size for visual consistency
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for TextInput usage in the codebase to compare prop patterns
ast-grep --pattern 'TextInput $props'

# Search for TextField usage to confirm consistent prop patterns
ast-grep --pattern 'TextField $props'

Length of output: 4435


Script:

#!/bin/bash
# Search for the specific file to see the previous implementation
rg -U "TextInput.*Search" app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/ -A 10

# Search for TextField usage in the table header context
rg -U "TextField.*Search" app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/ -A 10

Length of output: 225


Script:

#!/bin/bash
# Find the Search component file and its history
rg -U "Search\.tsx" app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/ -l

# Look for the content of the Search component
cat app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/Search.tsx

Length of output: 768


Script:

#!/bin/bash
# Get the git history of the Search component to see the previous implementation
git log -p app/client/src/modules/ui-builder/ui/wds/WDSTableWidget/component/TableHeader/Search.tsx

# Look for TextInput type definitions
rg -U "interface TextInputProps" app/client/src/

Length of output: 181


Script:

#!/bin/bash
# Look at the TextInput component implementation
cat app/client/src/components/designSystems/appsmith/TextInputComponent.tsx

# Check TextField type definitions from WDS
rg -U "interface TextFieldProps" app/client/src/

Length of output: 4387

app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarCell.tsx (2)

1-8: LGTM! Well-organized imports with clear aliasing.


23-23: LGTM! Clear and consistent export statement.

app/client/packages/design-system/widgets/src/components/Calendar/src/CalendarHeaderCell.tsx (2)

1-5: LGTM! Clean import organization.

Import statements are well-structured with clear separation of types.


22-22: LGTM! Proper named export.

Using named exports is the preferred approach for better tree-shaking and explicit imports.

app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget/component/index.tsx (2)

3-3: LGTM: Clean import update

The TextField import aligns with the component rename initiative.


Line range hint 19-35: Verify TextField component API compatibility

The component replacement looks correct, but let's verify that all props are compatible with the new TextField component.

✅ Verification successful

Let me analyze the output and generate appropriate follow-up commands to verify the props compatibility.


Props compatibility verified for TextField component

All props being passed to TextField component are compatible with the component's interface. The props used in the code (autoComplete, autoFocus, contextualHelp, errorMessage, isDisabled, isInvalid, isReadOnly, isRequired, label, onChange, onFocusChange, onKeyDown, placeholder, prefix, value) align with the TextField component's interface which extends both AriaTextFieldProps and FieldProps from the design system.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify TextField component props interface
# Look for TextField component definition and its props interface

ast-grep --pattern 'interface $name {
  $$$
}' | grep -A 20 "TextFieldProps"

Length of output: 76


Script:

#!/bin/bash
# Let's try a different approach to find the TextField component and its props
# First, let's locate the TextField component file
fd TextField

# Then let's look for the component definition and props
rg -A 10 "interface.*TextField.*Props" 

Length of output: 5035


Script:

#!/bin/bash
# Let's check the AriaTextFieldProps and FieldProps that TextFieldProps extends
rg -A 10 "interface (AriaTextFieldProps|FieldProps)" 

# Also let's check the actual usage of TextField in the component
rg -A 10 "import.*TextField" app/client/src/modules/ui-builder/ui/wds/WDSPhoneInputWidget/component/index.tsx

Length of output: 3562

app/client/packages/design-system/widgets/src/components/Calendar/stories/Calendar.stories.tsx (1)

17-18: LGTM!

Type definitions are correctly structured following Storybook conventions.

app/client/src/modules/ui-builder/ui/wds/WDSCurrencyInputWidget/component/index.tsx (1)

2-2: Verify consistent TextField migration across the codebase

The TextInput to TextField migration should be consistent across all components.

✅ Verification successful

Migration is correctly implemented

The TextField migration in this file is consistent with the codebase's patterns. The TextInput components found in the search results are from different packages or custom implementations, not from @appsmith/wds. The TextField from @appsmith/wds is being used appropriately in WDS (Widget Design System) components.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining TextInput usages that need migration

# Search for remaining TextInput imports from @appsmith/wds
rg "import.*TextInput.*from.*@appsmith/wds"

# Search for TextInput component usage
rg "<TextInput"

Length of output: 878


Script:

#!/bin/bash
# Let's verify the imports to understand the migration pattern

# Check TextInput imports to understand the source
rg "import.*TextInput.*from" -A 1

# Check TextField imports to understand the migration pattern
rg "import.*TextField.*from" -A 1

# Check actual TextField usage
rg "<TextField"

Length of output: 10943

app/client/packages/design-system/widgets/src/components/Calendar/src/Calendar.tsx (1)

29-36: Verify the grid implementation for all date formats.

Ensure the calendar grid handles various date formats and locales correctly.

app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx (3)

1-14: LGTM! Clean interface definition with proper typing.


16-18: LGTM! Clean component implementation.


19-32: Previous comment about redundant Boolean() calls is still applicable.

🧰 Tools
🪛 Biome (1.9.4)

[error] 20-20: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)


[error] 24-24: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)


[error] 29-29: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

app/client/packages/design-system/widgets/src/components/TimeField/src/TimeField.tsx (1)

1-20: LGTM! Well-organized imports.

The imports are properly structured and all dependencies are clearly defined.

app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx (1)

1-21: LGTM! Well-organized imports.

The imports are clean and properly structured, with good separation between internal and external dependencies.

app/client/packages/design-system/widgets/src/testing/ComplexForm.tsx (2)

19-19: LGTM: Import statement is correctly placed

The TextField import aligns with the component migration from TextInput.


170-170: LGTM: Consistent component sizing

The TextField size prop matches adjacent components, maintaining visual consistency.

Comment on lines +57 to +59
const root = document.body.querySelector(
"[data-theme-provider]",
) as HTMLButtonElement;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect type casting of root element

The root element with [data-theme-provider] selector might not be a button element.

- ) as HTMLButtonElement;
+ ) as HTMLElement;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const root = document.body.querySelector(
"[data-theme-provider]",
) as HTMLButtonElement;
const root = document.body.querySelector(
"[data-theme-provider]",
) as HTMLElement;

@jsartisan jsartisan requested a review from KelvinOm November 22, 2024 08:34
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (7)
app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts (1)

7-19: Consider enhancing type documentation and constraints.

The interface structure is solid, but could benefit from additional documentation:

  • Add @example usage in JSDoc
  • Document the available size options explicitly
  • Consider adding validation pattern for popoverClassName
 export interface DatePickerProps<T extends DateValue>
   extends Omit<SpectrumDatePickerProps<T>, "slot" | "placeholder">,
     FieldProps {
+  /**
+   * size of the select
+   * @default medium
+   * @example
+   * ```tsx
+   * <DatePicker size="medium" />
+   * ```
+   */
   size?: Omit<keyof typeof SIZES, "xSmall" | "large">;
-  /**
-   * className for the popover
-   */
+  /**
+   * className for the popover
+   * @example
+   * ```tsx
+   * <DatePicker popoverClassName="custom-popover" />
+   * ```
+   */
   popoverClassName?: string;
 }
app/client/packages/storybook/.storybook/preview-head.html (1)

32-34: Consider simplifying the link selector

While the current implementation works, the selector could be simplified to improve readability.

-  a:not(.docs-story a):not(.sb-unstyled a) {
+  a:not(.docs-story *):not(.sb-unstyled) {
app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx (2)

19-26: Add description and default props to Main story

The Main story should serve as a reference implementation. Consider adding:

  1. JSDoc description of the basic usage
  2. Default props that showcase typical usage
 export const Main: Story = {
   args: {
+    label: "Select Date",
+    placeholder: "MM/DD/YYYY",
   },
+  parameters: {
+    docs: {
+      description: {
+        story: 'Basic implementation of the DatePicker component with standard configuration.',
+      },
+    },
+  },
   render: (args) => (

108-133: Refactor Granularity story to reduce repetition

The story can be more maintainable by using array mapping instead of repeated component declarations.

 export const Granularity: Story = {
   render: () => (
     <Flex direction="column" gap="spacing-5" width="sizing-100">
-      <DatePicker
-        granularity="day"
-        label="Day"
-        popoverClassName="sb-unstyled"
-      />
-      <DatePicker
-        granularity="hour"
-        label="Hour"
-        popoverClassName="sb-unstyled"
-      />
-      <DatePicker
-        granularity="minute"
-        label="Minute"
-        popoverClassName="sb-unstyled"
-      />
-      <DatePicker
-        granularity="second"
-        label="Second"
-        popoverClassName="sb-unstyled"
-      />
+      {['day', 'hour', 'minute', 'second'].map((granularity) => (
+        <DatePicker
+          key={granularity}
+          granularity={granularity}
+          label={granularity.charAt(0).toUpperCase() + granularity.slice(1)}
+          popoverClassName="sb-unstyled"
+        />
+      ))}
     </Flex>
   ),
 };
app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx (3)

22-36: Consider adding JSDoc comments for better documentation.

The component definition and props destructuring look good. Adding JSDoc comments would improve developer experience.

Add documentation above the component:

/**
 * A date picker component that supports optional time selection.
 * @template T - Type extending DateValue
 * @param props - Component props
 */

37-46: Consider extracting type guard for time values.

The type assertions could be made safer by using a type guard function.

function isTimeValue(value: DateValue | null | undefined): value is TimeValue {
  return value != null && 'hour' in value;
}

const timePlaceholder = isTimeValue(placeholder) ? placeholder : null;
const timeMinValue = isTimeValue(props.minValue) ? props.minValue : null;
const timeMaxValue = isTimeValue(props.maxValue) ? props.maxValue : null;

90-104: Consider extracting TimeField rendering to a separate component.

The time field rendering logic could be moved to a separate component for better maintainability.

interface TimeFieldWrapperProps {
  timeGranularity: TimeValue['granularity'];
  state: typeof state;
  props: DatePickerProps<T>;
}

const TimeFieldWrapper = ({ timeGranularity, state, props }: TimeFieldWrapperProps) => (
  <div className={datePickerStyles.timeField}>
    <TimeField
      granularity={timeGranularity}
      hideTimeZone={props.hideTimeZone}
      hourCycle={props.hourCycle}
      label="Time"
      maxValue={timeMaxValue}
      minValue={timeMinValue}
      onChange={state.setTimeValue}
      placeholderValue={timePlaceholder}
      value={state.timeValue}
    />
  </div>
);
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e067549 and a1a552c.

📒 Files selected for processing (4)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx (1 hunks)
  • app/client/packages/storybook/.storybook/preview-head.html (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx

[error] 50-50: Avoid redundant Boolean call

It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call

(lint/complexity/noExtraBooleanCast)

🔇 Additional comments (9)
app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts (2)

1-5: LGTM! Clean imports with appropriate type definitions.

The imports are well-organized, pulling in necessary types from react-aria-components and the design system.


21-21: LGTM! Clean type re-export.

The DateValue type re-export is clean and follows best practices.

app/client/packages/storybook/.storybook/preview-head.html (3)

14-17: LGTM! Good isolation of table styles

The selective application of border colors using :not() ensures proper component isolation in Storybook.


19-22: LGTM! Consistent style isolation

Background color isolation matches the border color approach, maintaining consistency.


24-29: Verify browser compatibility for :has() selector

While the styling is comprehensive, the :has() selector might not be supported in all browsers. Consider adding a fallback.

✅ Verification successful

:has() selector is safe to use based on browser targets

The browserslist configuration shows modern browser targets (>0.2%, not dead), which aligns well with the current browser support for the :has() selector. All major browsers now support this feature, and the codebase explicitly excludes legacy browsers like IE11. No fallback is necessary.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any browser compatibility comments or issues
rg -i "browser.*(support|compat)" app/client/packages/design-system

Length of output: 69


Script:

#!/bin/bash
# Check browser compatibility comments in storybook config and related files
rg -i "browser.*(support|compat)" app/client/packages/storybook

# Check if :has selector is used elsewhere in the codebase
rg ":has\(" app/client/packages/storybook

# Look for any CSS fallback patterns or browser support utilities
rg "@supports" app/client/packages/storybook

Length of output: 154


Script:

#!/bin/bash
# Check the target browsers configuration in package.json or browserslist files
fd -e json package.json -x cat {} | jq -r '.browserslist'
fd browserslist -x cat {}

# Check for any browser support documentation in README files
fd -g "*README*" app/client/packages/storybook -x cat {}

Length of output: 349

app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx (2)

1-17: LGTM! Clean imports and proper meta setup.


94-106: Skipping comment as there's an existing valid review about the date range issue

app/client/packages/design-system/widgets/src/components/Datepicker/src/Datepicker.tsx (2)

1-21: LGTM! Clean imports with proper organization.

The imports are well-organized and all are being used in the component.


84-86: Review UNSTABLE_portalContainer usage.

The UNSTABLE_ prefix suggests this API might change. Consider adding a comment explaining why this specific implementation is necessary and track the API's stability status.

✅ Verification successful

Let me gather more context about the Popover component and its API.


Let me try to find any documentation or comments about this API.


Let me try one more search with ripgrep without file type filters.


UNSTABLE_portalContainer usage is consistent with design system patterns

The UNSTABLE_portalContainer prop is used consistently across multiple design system components (Menu, Select, ComboBox, Sheet, Datepicker) for portal container management. The Menu component even documents its usage with a clear comment explaining why it's necessary for correct functionality. Given this established pattern in the design system, no changes are needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for other usages and any documentation about this API
rg "UNSTABLE_portalContainer" -B 2 -A 2

Length of output: 3141


Script:

#!/bin/bash
# Check for Popover component implementation and documentation
ast-grep --pattern 'interface $_ {
  $$$
  UNSTABLE_portalContainer?: $_
  $$$
}'

Length of output: 83


Script:

#!/bin/bash
# Search for documentation or comments about UNSTABLE_portalContainer
rg -B 3 -A 3 "UNSTABLE_portalContainer" --type ts --type tsx --type js --type jsx --type md

Length of output: 119


Script:

#!/bin/bash
# Search for documentation or comments mentioning UNSTABLE_portalContainer
rg -B 5 -A 5 "UNSTABLE_portalContainer" --no-heading

Length of output: 6358

Comment on lines +67 to +85
export const Validation: Story = {
render: () => (
<form
onSubmit={(e) => {
e.preventDefault();
alert("Form submitted");
}}
>
<Flex direction="column" gap="spacing-5" width="sizing-60">
<DatePicker
isRequired
label="Validation"
popoverClassName="sb-unstyled"
/>
<Button type="submit">Submit</Button>
</Flex>
</form>
),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance Validation story with proper feedback

The current implementation could be improved to better demonstrate validation states:

  1. Replace alert with proper feedback UI
  2. Add examples of validation error states
 export const Validation: Story = {
   render: () => (
     <form
       onSubmit={(e) => {
         e.preventDefault();
-        alert("Form submitted");
+        // Use proper feedback UI instead of alert
+        const feedback = document.getElementById('feedback');
+        if (feedback) feedback.textContent = 'Form submitted successfully';
       }}
     >
       <Flex direction="column" gap="spacing-5" width="sizing-60">
         <DatePicker
           isRequired
           label="Validation"
           popoverClassName="sb-unstyled"
+          errorMessage="Please select a valid date"
         />
         <Button type="submit">Submit</Button>
+        <div id="feedback" role="status" />
       </Flex>
     </form>
   ),
+  parameters: {
+    docs: {
+      description: {
+        story: 'Demonstrates various validation states and error handling.',
+      },
+    },
+  },
 };

Committable suggestion skipped: line range outside the PR's diff.

@jsartisan jsartisan added the ok-to-test Required label for CI label Nov 22, 2024
@jsartisan jsartisan merged commit d87f7cc into release Nov 22, 2024
@jsartisan jsartisan deleted the chore/wds-datepicker branch November 22, 2024 10:20
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Nov 27, 2024
Added daterpicker component along with other components needed for it
like Calendar andTimeField

Datepicker
![CleanShot 2024-11-21 at 12 38
20@2x](https://github.com/user-attachments/assets/334cdbb9-10f8-442a-93d8-c62b83668972)

Calendar
![CleanShot 2024-11-21 at 12 38
54@2x](https://github.com/user-attachments/assets/6123a842-5c32-48f1-86f9-56ffe249fedf)

Timefield
![CleanShot 2024-11-21 at 12 41
56@2x](https://github.com/user-attachments/assets/b9b2dc3c-91fd-4b90-99f4-461685be7b37)

/ok-to-test tags="@tag.Anvil"

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Release Notes

- **New Features**
	- Introduced a `Calendar` component for date selection and display.
- Added a `DatePicker` component for selecting dates and times with
enhanced error handling.
- Launched a `TimeField` component for time input with optional prefix
and suffix.
- Updated `TextField` component replacing the previous `TextInput` for
improved usability.

- **Bug Fixes**
	- Enhanced styling and responsiveness of input components.

- **Documentation**
- Added Storybook stories for `Calendar`, `DatePicker`, and `TimeField`
components to showcase functionalities and configurations.

- **Chores**
- Refactored imports to utilize the new `TextField` component across
various widgets.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11970103158>
> Commit: a1a552c
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11970103158&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Anvil`
> Spec:
> <hr>Fri, 22 Nov 2024 10:01:23 UTC
<!-- end of auto-generated comment: Cypress test results  -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants