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

fix: check for valid date in frontend #113

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

fmartingr
Copy link
Contributor

Summary

Changes the onChange function for date files and resets the field in the date set up is invalid.

Tried adding a HTMLInput.setCustomValidity method but our form does now follow that flow it seems, hence opted for not allowing the user to have an invalid input set in the field (which should not happen very frequently).

Fixes #111

@fmartingr fmartingr added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Sep 24, 2024
@fmartingr fmartingr self-assigned this Sep 24, 2024
@wiggin77
Copy link
Member

@fmartingr an update to CI was just merged. Update this PR from master and it should be passing.

Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@wiggin77 wiggin77 removed the 2: Dev Review Requires review by a core committer label Sep 24, 2024
@AayushChaudhary0001
Copy link

@fmartingr I have tested this PR and still I am to reproduce the actual issue, please check it thorugh the screenshots below. The date which does not even exist is getting saved and the end date shown as result is never, which would result in never ending process.

image

image

@fmartingr
Copy link
Contributor Author

I can't seem to reproduce this. Can you record a video?

Screen.Recording.2024-10-02.at.15.41.52.mov

@wiggin77
Copy link
Member

wiggin77 commented Oct 2, 2024

I can't seem to reproduce this. Can you record a video?

@AayushChaudhary0001

@AayushChaudhary0001
Copy link

@fmartingr Please check the issue in this video:-

date_validation.mp4

@fmartingr
Copy link
Contributor Author

I have been digging a bit more about this, and I'm not sure it's worth fixing for invalid dates.

Essentially, an input type date does not store the value if the date is an invalid date 1, so the code can't check easily what's on the input when a full invalid date has been entered, since the value is going to be empty.

Adding to that, we allow empty values in that field, so the submit event is valid when we enter an invalid date.

I'm not sure this is going to happen that much, so I'm not sure it's worth dedicating more time to it.

What I'm not sure on is how to handle from a UX perspective that 1% of the time that is going to happen.

cc: @wiggin77

@wiggin77
Copy link
Member

wiggin77 commented Oct 3, 2024

@fmartingr let's defer this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: QA Review Requires review by a QA tester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changes in date validation contains some issue
3 participants