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

chore: add loose restrictions to date time text field #7024

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

Conversation

Xazin
Copy link
Contributor

@Xazin Xazin commented Dec 20, 2024

Closes: #6788

This just adds some loose restrictions to the Date and Time text fields in the Date Picker.

This is done to help the user write the correct format. It's not completely enforced according to the date format, but does limit the user a bit more.

24-hours - Can only enter 5 characters (12:34) and only enter numbers and :
12-hours - Can enter AM and PM as well (12:34 AM) 8 characters

Date field - Can only enter 12 characters (Unsure if should limit it to 10) and allows ., -, and / characters other than numbers.

PR Checklist

  • My code adheres to AppFlowy's Conventions
  • I've listed at least one issue that this PR fixes in the description above.
  • I've added a test(s) to validate changes in this PR, or this PR only contains semantic changes.
  • All existing tests are passing.

@richardshiue
Copy link
Collaborator

richardshiue commented Dec 30, 2024

Hmm... I'm not sure I actually like this change. For some users, this helps to give an input that will be correctly parsed. On the other hand, for others (or it's me only) this could be a hindrance. If I put an extra space somewhere in the input, I could hit the length limit and have to go back and correct that before entering my last few characters.

Also, this prevents users from entering dates in the way that is most comfortable to them. The unit tests are failing since the tests are using human-readable date inputs (Nov 30, 2024) to take advantage of the date parsing library we're using

@Xazin
Copy link
Contributor Author

Xazin commented Dec 30, 2024

Hmm... I'm not sure I actually like this change. For some users, this helps to give an input that will be correctly parsed. On the other hand, for others (or it's me only) this could be a hindrance. If I put an extra space somewhere in the input, I could hit the length limit and have to go back and correct that before entering my last few characters.

Also, this prevents users from entering dates in the way that is most comfortable to them. The unit tests are failing since the tests are using human-readable date inputs (Nov 30, 2024) to take advantage of the date parsing library we're using

I see what you mean, but how would users know they can type Nov 30, 2024? We don't have a way for them to discover this right?

The hint in the textfield show the "correct" format, so users would assume that to be followed.

We can remove the formatter for the Date field if you prefer?

@richardshiue
Copy link
Collaborator

richardshiue commented Dec 31, 2024

We don't have a way for them to discover this right?

You're right. We don't need to though, since if we let users enter dates however they want and we parse them correctly every time, it'll give them flexibility and convenience.

We can remove the formatter for the Date field if you prefer?

Yes I think that would be better

@Xazin
Copy link
Contributor Author

Xazin commented Jan 4, 2025

We don't have a way for them to discover this right?

You're right. We don't need to though, since if we let users enter dates however they want and we parse them correctly every time, it'll give them flexibility and convenience.

We can remove the formatter for the Date field if you prefer?

Yes I think that would be better

Done, sorry about the delay :)

@Xazin Xazin force-pushed the chore/loose-restrictions-date-picker branch from dcc6374 to ed77b76 Compare January 4, 2025 22:43
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 this pull request may close these issues.

[FR] Add TextFormatter to the time text field in date picker
2 participants