-
-
Notifications
You must be signed in to change notification settings - Fork 747
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
Comments
Hi @anthony-bonta-gaf-energy |
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? |
@rodgobbi the use case for this is as follows:
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. |
Agree with @anthony-bonta-gaf-energy, better not have any side effects caused by mounting the For me, it's still a valid approach to show the outside days as disabled, need to check @gpbl's thoughts. @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 If the valid date range changes when loading the form, and the previous selected date is invalid, this could be considered an invalid input. |
@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| 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. |
Thanks for your feedbacks here @anthony-bonta-gaf-energy @rodgobbi! I understand that a selected date outside the ![]() Some thoughts here:
|
@anthony-bonta-gaf-energy your idea is great, it's very clear to the user that they need to select a new date.
@gpbl minor correction, having an invalid selected date doesn't break the calendar, but what is breaking in the example is using a
That makes sense, I'll open a PR for it. |
@rodgobbi thanks for your PR #2672! That one should fix the bug. Will release later a v9.5.1.
I’ve considered displaying console errors when |
IMO this pattern is useful, it helps developers troubleshoot issues way quicker. We could assume that developers will notice that the 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 " 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 |
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 |
Indeed, it is good idea @gpbl, and thanks for the feedback 👍 |
Marking this as closed by #2672. |
To reproduce
If you’re unable to easily reproduce this bug, consider opening a support thread to discuss it further.
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
The text was updated successfully, but these errors were encountered: