tremorlabs / tremor

React components to build charts and dashboards
https://tremor.so
Apache License 2.0
15.65k stars 452 forks source link

Datepicker doesn't work if values are relying on state outside component since whole component rerenders #243

Closed maakle closed 1 year ago

maakle commented 1 year ago

So I am facing the following issue. I am fetching data from my API via a hook (in the code useHeadcount). I am using Nextjs & SWR for fetching. This hook needs the startDate and endDate from tremor's Datepicker to query the API for the headcount in that range. I am controlling those outside with a simple useState. Whenever a user selects the dates from the Datepicker component, the handleFilterDate is called and updates the variables.

The issue that arrises here is that the whole component is mounted and re-rendered, whenever I select something from the datepicker, resetting startDate and endDate back to the default values I set. This results that a user is unable to select a date range, always going back to the defaults. The behavior can be seen in the screen recording here.

I tried to use useCallback (on handleFilterDate) and useMemo (on the DatePicker component) but unsuccessfully. The only ugly solution was to refactor the whole component, making Datepicker not relying on data, isLoading or error and due to the rerender. I'll paste the ugly solution below.

There has to be a better & cleaner way how to solve this. Any ideas what to do that? The ugly solution can't be the only solution...

Code:

const HeadcountCard = () => {
  const { workspace } = useWorkspace();

  const currentYear = new Date().getFullYear();
  const [startDate, setStartDate] = useState(new Date(currentYear, 0, 1));
  const [endDate, setEndDate] = useState(new Date());

  const { isLoading, isError, data } = useHeadcount(
    workspace.slug,
    startDate,
    endDate,
  );

  // TODO: Right now there is a bug that it alwaya resets the date again to original startDate &  endDate
  const handleFilterDate = (sDate: Date, eDate: Date) => {
    setStartDate(sDate);
    setEndDate(eDate);
  };

  if (isLoading) {
    return <LoadingMetricCard />;
  }

  if (isError) {
    return <DataFetchingErrorCard />;
  }

  return (
    <MetricCard>
      <div className="flex justify-between items-center">
        <div className="flex flex-row items-center space-x-10">
          <div className="flex flex-row space-x-2">
            <div className="text-xl font-semibold dark:text-white">
              Headcount
            </div>
            <div className="text-xl font-bold">{data.headcount.length}</div>
          </div>
          <div className="flex flex-row items-center space-x-3">
            <BadgeDelta deltaType={"increase"} text={"23.9%"} />
            <div className="text-sm text-stone-400">YTD</div>
          </div>
        </div>

        <div>
          <Link
            className="text-blue-600"
            href={`/dashboard/${workspace.slug}/metrics/headcount`}
          >
            Details &rarr;
          </Link>
        </div>
      </div>
      <div className="space-y-8">
        <AreaChart
          data={data.headcount}
          categories={["employees"]}
          dataKey="date"
          height="h-72"
          colors={["cyan", "indigo"]}
          marginTop="mt-4"
        />
        <Datepicker
          placeholder="Select..."
          enableRelativeDates={true}
          handleSelect={handleFilterDate}
          defaultRelativeFilterOption="y"
          defaultStartDate={null}
          defaultEndDate={null}
          minDate={null}
          maxDate={null}
          color="blue"
          maxWidth="max-w-none"
          marginTop="mt-0"
        />
      </div>
    </MetricCard>
  );
};

export default HeadcountCard;

Ugly solution that works...

function HeadcountCardPartial({
  slug,
  headcount,
  loading,
  error,
}: {
  slug: string;
  headcount: [
    {
      interval: number;
      date: Date;
      employees: number;
    },
  ];
  loading: boolean;
  error: Error;
}) {
  if (loading) {
    return <LoadingMetricCard />;
  }

  if (error) {
    return <DataFetchingErrorCard />;
  }

  return (
    <>
      <div className="flex justify-between items-center">
        <div className="flex flex-row items-center space-x-10">
          <div className="flex flex-row space-x-2">
            <div className="text-xl font-semibold dark:text-white">
              Headcount
            </div>
            <div className="text-xl font-bold">{headcount.length}</div>
          </div>
          <div className="flex flex-row items-center space-x-3">
            <BadgeDelta deltaType={"increase"} text={"23.9%"} />
            <div className="text-sm text-stone-400">YTD</div>
          </div>
        </div>

        <div>
          <Link
            className="text-blue-600"
            href={`/dashboard/${slug}/metrics/headcount`}
          >
            Details &rarr;
          </Link>
        </div>
      </div>
    </>
  );
}

function HeadcountCard() {
  const { workspace } = useWorkspace();

  const currentYear = new Date().getFullYear();
  const [startDate, setStartDate] = useState(new Date(currentYear, 0, 1));
  const [endDate, setEndDate] = useState(new Date());

  const { isLoading, isError, data } = useHeadcount(
    workspace.slug,
    startDate,
    endDate,
  );

  const handleFilterDate = (sDate: Date, eDate: Date) => {
    setStartDate(sDate);
    setEndDate(eDate);
  };

  return (
    <MetricCard>
      <HeadcountCardPartial
        slug={workspace.slug}
        headcount={data?.headcount}
        loading={isLoading}
        error={isError}
      />

      <div className="space-y-8">
        {!isLoading && (
          <AreaChart
            data={data?.headcount}
            categories={["employees"]}
            dataKey="date"
            height="h-72"
            colors={["cyan", "indigo"]}
            marginTop="mt-4"
          />
        )}

        <Datepicker
          placeholder="Select..."
          enableRelativeDates={true}
          handleSelect={handleFilterDate}
          defaultRelativeFilterOption="y"
          defaultStartDate={null}
          defaultEndDate={null}
          minDate={null}
          maxDate={null}
          color="blue"
          maxWidth="max-w-none"
          marginTop="mt-0"
        />
      </div>
    </MetricCard>
  );
}

export default HeadcountCard;
ilin-andrey commented 1 year ago

I afraid that your "ugly" approach is the only approach which works in your case. Datepicker component doesn't allow to define state, it is uncontrolled component, on re-render you unmount it from DOM and then mount it back as a fresh component with same default values.

maakle commented 1 year ago

I just tried this and added startDate and endDate to defaultStartDate and defaultEndDate of the component. But no success. Still same issue and the whole page rerenders setting back the variables to their default. Maybe @ilin-andrey had more luck?

ilin-andrey commented 1 year ago

Your problem is here @mathiasklenk, defaultRelativeFilterOption option overrides defaultStartDate/defaultEndDate even if you set correct values. This component with enabled defaultRelativeFilterOption renders same values. Look here https://github.com/tremorlabs/tremor/blob/main/src/components/input-elements/Datepicker/Datepicker.tsx#L476 for more details.

Screenshot 2022-12-14 at 22 47 19

maakle commented 1 year ago

Okay wow that seem to have done the trick @ilin-andrey. Now the datepicker does correctly work and does not reset to the defaults. Is this overwrite intended behavior @mitrotasios? In my specific case I would actually prefer to have the relative filter dates as defaults but could live with the behavior now.

Side note: showYearPagination seems to have been renamed to enableYearPagination={true} but doesn't work. At least I didn't see any UI change and buttons where I can go through years. Perhaps also add that to the issue tracker & update docs. :)

mitrotasios commented 1 year ago

Oh thanks @ilin-andrey, good catch! @mathiasklenk, yeah I think it makes more sense to give the defaultStartDate and defaultEndDate precedence over the defaultRelativeFilterOption. Will change this in the next release.

It seems like we have a mistake in our docs with the year pagination, thanks! Will fix ASAP.

mitrotasios commented 1 year ago

JFYI, we are currently working on adding the ability for our input components to be controlled, i.e. to control the state from the parent.

mitrotasios commented 1 year ago

@mathiasklenk we just shipped a new DateRangePicker component in v1.4.0 that is supposed to replace the Datepicker by addressing these issues 😊 @all let us know if you encounter any more issues

mitrotasios commented 1 year ago

re-opening for visibility

angelhodar commented 1 year ago

@mitrotasios Hey, I am trying the new DateRangePicker in a controlled way and the problem is that the onValueChange is fired 2 times. First I have to select the start date, then onValueChange is called with second element for end date as null, and when selected another date then its called again with start and end date setup. The problem is that I expect the component to only call onValueChange once the 2 dates are selected like you have in the documentation of the component but I cant get to that situation. Any suggestions?

mitrotasios commented 1 year ago

Hey @angelhodar, this behavior is expected and somewhat necessary to enable controllable state unfortunately. What I suggest is that you trigger any actions that depend on the selected date range (e.g. filtering some other data) only if both dates are provided / not null.