Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting the display month to a value outside of the selectable range causes the DayButton component to not render #2671

Closed
anthony-bonta-gaf-energy opened this issue Jan 22, 2025 · 12 comments · Fixed by #2672

Comments

@anthony-bonta-gaf-energy
Copy link

anthony-bonta-gaf-energy commented Jan 22, 2025

To reproduce

  1. Set both startMonth and endMonth to a range value
  2. Set the month prop to a value outside of the startMonth and endMonth
  3. Render the component

If you’re unable to easily reproduce this bug, consider opening a support thread to discuss it further.

import { useMemo, useState } from 'react';
import { addYears, startOfYear, endOfYear } from 'date-fns';
import { DayPicker } from 'react-day-picker';

export function App() {
  const today = useMemo(() => new Date(), []);
  const oldDays = useMemo(() => addYears(today, -10), [today]);
  const [month, setMonth] = useState(oldDays);
  const yearsFuture = endOfYear(addYears(today, 2));
  const yearsAgo = startOfYear(addYears(today, -2));

  return (
    <DayPicker
      captionLayout='dropdown'
      endMonth={yearsFuture}
      month={month}
      onMonthChange={setMonth}
      mode='single'
      hideNavigation
      required={false}
      selected={oldDays}
      startMonth={yearsAgo}
      style={{ backgroundColor: 'white' }}
    />
  );
}

CodeSandbox: https://playcode.io/2231858

Actual Behavior

The table cells render for the month and the actual months in the drop down are correctly disabled. However, the buttons in each cell do not render at all.

Note that you can still work around this by selecting a year in the range.

Expected Behavior

I would expect a set of buttons to be rendered even if they're outside of the range. The use case for this is a range of +-1 year and the user selected a value over a year ago and we want to reflect that in the selection and make them update it.

Screenshots

Image

@rodgobbi
Copy link
Collaborator

Hi @anthony-bonta-gaf-energy
Even if the day buttons would be displayed outside the valid range, they would be displayed disabled, besides all the month options being disabled, which can feel confusing for a user, and in the behavior you described it's expected that the user should select an year that is already selected by default in the dropdown to enable the calendar for selection, therefore the steps to put the DayPicker in a valid month is very hidden.
Is there a specific reason for this use case?
Wouldn't it be better to programmatically put the DayPicker in a state where the user can start selecting valid dates?

@gpbl
Copy link
Owner

gpbl commented Jan 23, 2025

I agree that this is an unexpected state of the calendar.

Should DayPicker handle this case, or should the developer take care of the values passed in? Should the days be clickable - or should we render the closest available month?

@anthony-bonta-gaf-energy
Copy link
Author

anthony-bonta-gaf-energy commented Jan 23, 2025

@rodgobbi the use case for this is as follows:

  1. Calendar is set so that only the current year, previous year, and next year are valid dates to update to.
  2. User enters a value 2 years ago which was valid at the time.
  3. User needs to update this value today
  4. User goes back to the form and views the calendar but now it's empty and they don't see the previous date they entered.

They can correct it by selecting a year in the range, but it would be nice to show a read-only view of the previous date they selected. This is a data flow issue. When the data comes in, the drop downs correctly reflect the old date, but it doesn't show the day.

@gpbl The main concern with trying to auto correct this is that you'll be lying to the user about the value that is actually set. If data is flowing in a single direction, as it should be, the value coming into the calendar is a valid date but needs to be corrected. I want to be transparent with the user about what the current value is and force them to tell me what to update it to. If you show them the latest month, many will get confused and think the currently selected day is not what they originally chose.

@rodgobbi
Copy link
Collaborator

Agree with @anthony-bonta-gaf-energy, better not have any side effects caused by mounting the DayPicker.

For me, it's still a valid approach to show the outside days as disabled, need to check @gpbl's thoughts.
For the question "Should the days be clickable", the days would be at least disabled because they are outside the valid range.
For "should we render the closest available month?", by detault the DayPicker does that, but in the example in this issue the month is explicitly set, so the DayPicker uses it, which is correct IMO. After selecting a valid month through the dropdown the DayPicker goes back to a visible month.

@anthony-bonta-gaf-energy I have the feeling that just displayeing the day picker all disabled wouldn't be providing enough clarity to the users. The day picker only got to the inconsistent state because the month prop is being explicitly set to an invalid value, which the day picker wouldn't get there with the normal flow.

If the valid date range changes when loading the form, and the previous selected date is invalid, this could be considered an invalid input.
What do you think of displaying the day picker starting from a valid month, as it is the expected behavior, but displaying an error message / validation error for the input saying the previously selected date is not valid anymore?

@anthony-bonta-gaf-energy
Copy link
Author

anthony-bonta-gaf-energy commented Jan 24, 2025

@rodgobbi Tough call. My mental model for this would be something like a fallback state where if the month is explicitly set to something outside of the range, then instead of rendering the calendar, render a fallback component or some kind of invalid state.

|Su|Mo|Tu|We|Th|Fr|Sa|
|----------------------|
| Current selected month |
| and year is outside the |
| current valid range. |

Something like this would make it clear that the user has to do something.

The other possibility is to just render empty cells for the days and just show the selected date in the position that it would be in with the error message in front of the footer.

@gpbl
Copy link
Owner

gpbl commented Jan 25, 2025

Thanks for your feedbacks here @anthony-bonta-gaf-energy @rodgobbi!

I understand that a selected date outside the startMonth/endMonth causes DayPicker to display a broken calendar:

Image

Some thoughts here:

  • Developers are responsible for using the right props. They should test that they are passing valid values to DayPicker.
  • Invalid props should anyway put the calendar in a usable state. Users should be able to navigate and interact with it.
    • In this case, I’d ignore the invalid prop and render the calendar from the initial month.
  • I’d exclude error handling or console logs. They may require translations, break the user’s experience, or be missed by the developer.
  • I’d start adding some examples (we can begin with @anthony-bonta-gaf-energy’s code) and write the tests with the expected behavior.

@rodgobbi
Copy link
Collaborator

@anthony-bonta-gaf-energy your idea is great, it's very clear to the user that they need to select a new date.
To add to your idea: you could add a button that updates the month state when the user clicks and displays the calendar starting from a valid month, or any other form of call to action
You can customize the calendar more with Custom Components and reuse part of the default components when it's initially invalid, then switch to use all default components after the user interacts with the error message and is ready to select a new date.

I understand that a selected date outside the startMonth/endMonth causes DayPicker to display a broken calendar:

@gpbl minor correction, having an invalid selected date doesn't break the calendar, but what is breaking in the example is using a month prop value that is invalid.

In this case, I’d ignore the invalid prop and render the calendar from the initial month.

That makes sense, I'll open a PR for it.
I'm used to libs using console.error to alert developers when there's something wrong, in this case, it would be that the month prop value is not valid and will be ignored.
@gpbl do you think it's useful to use this pattern in the project?

@gpbl
Copy link
Owner

gpbl commented Jan 25, 2025

@rodgobbi thanks for your PR #2672! That one should fix the bug. Will release later a v9.5.1.

I'm used to libs using console.error to alert developers when there's something wrong, in this case, it would be that the month prop value is not valid and will be ignored.

I’ve considered displaying console errors when NODE_ENV=development in the past, but I don’t see how they would prevent these invalid cases.

@rodgobbi
Copy link
Collaborator

I’ve considered displaying console errors when NODE_ENV=development in the past, but I don’t see how they would prevent these invalid cases.

IMO this pattern is useful, it helps developers troubleshoot issues way quicker.

We could assume that developers will notice that the month and defaultMonth should be within the valid range. Still, if it's not explicitly stated in the docs and only enforced in the source code, I imagine some developers may get confused wondering why the month or defaultMonth values being passed are being ignored.

I don't know how useful the console errors would be for this particular use case we are discussing, but using it as an example for the general idea, we cannot completely prevent it, but the console errors can still help the developers troubleshoot issues the lib detected, and preventing would be the developer's responsibility in the end.

We can use React as a reference for the discussion.

For example, React has a validateDOMNesting function that validates the DOM structure, and it's common for me to see the console error message alerting that "a or button cannot be a descendant of a".
This is the developer's responsibility and cannot be prevented by React, but React still alerts about it.

Another console error message example that is close to our use case, React has console error messages to help developers properly use React's API, like the Missing "key" prop for element in iterator or Received "true"/"false" for a non-boolean attribute.
These issues cannot be completely prevented by React, but it handles them internally and uses a console error message to guide the developers to fix them.

@gpbl
Copy link
Owner

gpbl commented Jan 26, 2025

I value your opinion here, @rodgobbi, thanks! Open to adding better “error handling” to DayPicker!

Let’s see if we receive more feedback similar to this one. We should tolerate some quirks until we get a clear direction. For now, better update the documentation with a clear warning when using these start/endMonth props.

@rodgobbi
Copy link
Collaborator

rodgobbi commented Jan 27, 2025

Indeed, it is good idea @gpbl, and thanks for the feedback 👍
Let's keep the console error idea in mind when reading new issues or discussions that are opened and evaluate if a console error would be helpful for that use case and would help troubleshoot it.

@gpbl
Copy link
Owner

gpbl commented Feb 8, 2025

Marking this as closed by #2672.

@gpbl gpbl closed this as completed Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants