-
-
Notifications
You must be signed in to change notification settings - Fork 828
Conversation
@@ -85,6 +85,7 @@ const JumpToDatePicker: React.FC<IProps> = ({ ts, onDatePicked }: IProps) => { | |||
onInput={onDateValueInput} | |||
onKeyDown={onDateInputKeyDown} | |||
value={dateValue} | |||
max={(new Date()).toISOString().substring(0, 10)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to craft dateDefaultValue
above we do something a bit different. We should align whatever we choose here with that.
new Date().toISOString().substring(0, 10)
- This does not work when the year is
99000
: (new Date('99000-05-05').toISOString().substring(0, 10)
->'+099000-05'
) - This also feels ripe for the same problem that user-agent parsers had when Chrome went from
99
to100
- This does not work when the year is
new Date().toISOString().split('T')[0]
- This does better when the year is
99000
but it's not in the format that the date<input>
wants:new Date('99000-05-05').toISOString().split('T')[0]
->'+099000-05-05'
- This does better when the year is
Those options are considerably simpler than the current code but this always does the right thing. We probably just need to package this logic up into a function at the top called formatDateForInput
and re-use it in both places.
const date = new Date(ts);
const year = date.getFullYear();
const month = `${date.getMonth() + 1}`.padStart(2, "0");
const day = `${date.getDate()}`.padStart(2, "0");
const dateDefaultValue = `${year}-${month}-${day}`;
@@ -85,6 +85,7 @@ const JumpToDatePicker: React.FC<IProps> = ({ ts, onDatePicked }: IProps) => { | |||
onInput={onDateValueInput} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aman-godara Is this PR meant to be in draft? Draft means you're still working on it and it's not ready for review yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to test this on my local machine, hence I keep it under draft. The coding part is done unless testing fails.
Closing PR |
Fixes: 20800
Here's what your changelog entry will look like:
🐛 Bug Fixes