Implemented Date Range Input. #2328

shashankvish0010 · 2024-04-28T18:10:34Z

Description

Added a date range input component using shadcn/ui date picker component and Calendar component to select the dates.

Related issues

Fixes: #2270

Breaking changes

The Home page is updated with the date range section, which allows the user to select two dates, 'from' & 'to'.
Once the user selects the date, the range is updated within the search parameters as well.

changeset-bot · 2024-04-28T18:10:38Z

⚠️ No Changeset found

Latest commit: 34ed401

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderabbitai · 2024-04-28T18:10:48Z

Important

Auto Review Skipped

Auto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

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>.
- 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 generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @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.

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 as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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.

apps/backoffice-v2/src/common/hooks/useHomeLogic/useHomeLogic.ts

Omri-Levy · 2024-04-29T13:56:20Z

apps/backoffice-v2/src/pages/Home/Home.page.tsx

+          </h3>
+        </div>
+        <DateRangePicker
+          onChange={range => handleDateRangeChange(range)}


Suggested change

onChange={range => handleDateRangeChange(range)}

onChange={handleDateRangeChange}

apps/backoffice-v2/src/common/components/organisms/Calendar/Calendar.tsx

apps/backoffice-v2/src/common/components/molecules/DateRangePicker/DateRangePicker.tsx

Omri-Levy · 2024-04-29T14:22:30Z

apps/backoffice-v2/src/common/components/molecules/DateRangePicker/DateRangePicker.tsx

+  useEffect(() => {
+    if (date !== prevDateRef.current) {
+      onChange({
+        start: date?.from || null,
+        end: date?.to || null,
+      });
+      prevDateRef.current = date;
+    }
+  }, [date, onChange]);


useEffect should be a last resort. Here passing the date from outside as the useState's initial value and using the onChange on the component itself would behave in a more robust way and more consistently. Check this if you haven't yet. https://react.dev/learn/you-might-not-need-an-effect

apps/backoffice-v2/src/common/components/molecules/DateRangePicker/DateRangePicker.tsx

Omri-Levy · 2024-05-03T16:32:54Z

apps/backoffice-v2/src/pages/Home/Home.page.tsx

+            start: from ? new Date(from) : null,
+            end: to ? new Date(to) : null,
+          }}
+          className={'<div>'}


Seems like a mistake.

Omri-Levy · 2024-05-03T16:39:57Z

apps/backoffice-v2/src/common/components/molecules/DateRangePicker/DateRangePicker.tsx

+  const currentDate = new Date();
+  const [date, setDate] = useState<DateRange | undefined>({
+    from: new Date(currentDate.getFullYear(), currentDate.getMonth(), currentDate.getDate()),
+    to: dayjs(new Date(currentDate.getFullYear(), currentDate.getMonth(), currentDate.getDate(), 1))
+      .add(1, 'month')
+      .toDate(),
+  });
+
+  const handleDateChange = (selection: DateRange | undefined) => {
+    setDate(selection);
+    onChange(selection);
+  };


I was expecting these lines to be removed and for the component to receive onChange and value as props. 🙂

Suggested change

const currentDate = new Date();

const [date, setDate] = useState<DateRange | undefined>({

from: new Date(currentDate.getFullYear(), currentDate.getMonth(), currentDate.getDate()),

to: dayjs(new Date(currentDate.getFullYear(), currentDate.getMonth(), currentDate.getDate(), 1))

.add(1, 'month')

.toDate(),

});

const handleDateChange = (selection: DateRange | undefined) => {

setDate(selection);

onChange(selection);

};

Omri-Levy · 2024-05-03T16:41:48Z

apps/backoffice-v2/src/common/hooks/useHomeLogic/useHomeLogic.ts

+  const handleDateRangeChange: ComponentProps<typeof DateRangePicker>['onChange'] = (range: {
+    start: { toISOString: () => string };
+    end: { toISOString: () => string };
+  }) => {


ComponentProps['onChange'] gives you the type for range. If the type is wrong then we're doing something incorrectly.

Suggested change

const handleDateRangeChange: ComponentProps<typeof DateRangePicker>['onChange'] = (range: {

start: { toISOString: () => string };

end: { toISOString: () => string };

}) => {

const handleDateRangeChange: ComponentProps<typeof DateRangePicker>['onChange'] = (range) => {

Omri-Levy · 2024-05-07T09:32:39Z

apps/backoffice-v2/src/common/components/molecules/DateRangePicker/DateRangePicker.tsx

+
+export const DateRangePicker = ({ onChange, value, className }: TDateRangePickerProps) => {
+  const [date, setDate] = useState<DateRange | undefined>();
+  console.log(value);


Suggested change

console.log(value);

I just used that for debugging, to check if the value is being updated or not. I should exclude that part, but I have made a stupid mistake. I will keep that in mind next time.

Omri-Levy · 2024-05-07T09:33:31Z

apps/backoffice-v2/src/common/components/molecules/DateRangePicker/DateRangePicker.tsx

+            initialFocus
+            mode="range"
+            selected={date}
+            onSelect={(selection: DateRange | undefined) => {


Why are you overriding the onSelect type here?

Omri-Levy · 2024-05-07T09:34:50Z

apps/backoffice-v2/src/common/components/molecules/DateRangePicker/DateRangePicker.tsx

+            selected={date}
+            onSelect={(selection: DateRange | undefined) => {
+              setDate(selection);
+              onChange ?? onChange(selection);


You meant the opposite &&.

Suggested change

onChange ?? onChange(selection);

onChange?.(selection);

Omri-Levy · 2024-05-07T09:37:20Z

apps/backoffice-v2/src/common/components/molecules/DateRangePicker/DateRangePicker.tsx

+};
+
+export const DateRangePicker = ({ onChange, value, className }: TDateRangePickerProps) => {
+  const [date, setDate] = useState<DateRange | undefined>();


Why are we not using value over this state?

The reason for not using the value prop is because the value is the initial value of the date range, whenever we select a date range in the Calendar component, the value prop is not updated automatically. Instead, we need to update a component state to reflect the new selected date range. Moreover, I tried to use the value prop but on debugging I addressed that the date range set to from and to within the value prop results in undefined just after selecting the date range, it may be an issue in development mode because of React strict mode ?

The value should update since onChange is passed. onChange should not be optional. If value was used as initial I'd expect it to be passed into the useState.

If we were to make value optional (not saying we should) it would be fine to pass it into the useState as well since its already initialized with undefined.

I don't understand why we are passing value and making it required in the interface but it doesn't do anything. I do see now its used for a conditional class.

If value is from the URL and onChange updates the URL I'm failing to understand why it wouldn't work. 🙂

Yes, I completely understand and agree with you, the issue with the value is that it remains undefined even after selecting the date range, due to which {!value?.from && !value?.to && <span>Pick a date</span>} always remains active even after selecting the date range.

And as the type of onChange is SelectRangeEventHandler | undefined here, we cannot invoke an object that is possibly undefined thus need to use optional chaining.

Addressed in 38644b7

…a within home-page

Omri-Levy · 2024-05-11T09:54:08Z

apps/backoffice-v2/src/common/hooks/useHomeLogic/useHomeLogic.ts

+  const [{ from, to }, setSearchParams] = useZodSearchParams(HomeSearchSchema);
+  const { data: session } = useAuthenticatedUserQuery();
+  const { firstName, fullName, avatarUrl } = session?.user || {};
+  const queryParams = from || to ? `?from=${from}&to=${to}` : '';


This one is redundant. Overtime search query params will change, might as well pass the full search prop from useLocation like done in Cases.Item.tsx.

Omri-Levy · 2024-05-11T09:55:51Z

apps/backoffice-v2/src/pages/Home/Home.page.tsx

+      <div>
+        <Tabs defaultValue={pathname} key={pathname}>
+          <TabsList>
+            <TabsTrigger asChild={true} value={`/${locale}/home/statistics`}>


Is the active tab state affected by the search query param change? Do we need to also pass search into value?

Addressed in 6f4033d

Added: date range input component using ShadCN's date picker component

5425cd3