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

Day field improvements: Fixes #502, Fixes #497 #531

Merged
merged 4 commits into from
Aug 15, 2018

Conversation

febbraiod
Copy link
Member

No description provided.


if (date < minDate) date = minDate;
if (date > maxDate) date = maxDate;

Copy link
Member Author

@febbraiod febbraiod Aug 14, 2018

Choose a reason for hiding this comment

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

Debatable if this is the correct incremental step. I could also see just returning to keeping the previous value. My thought here is that one might be entering some arbitrary 'really long time ago' or 'really long time from now' date that is meant to mean 'show me everything'.

Ext actually displays the invalid date in this case but shows an inline validation failure. Which Lee mentioned we are headed toward.

Copy link
Member

Choose a reason for hiding this comment

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

I agree its debateable what the right things to do is, but we definitely need to do something, and this seems like a good first step.

We can revisit shortly during our validation work

@lbwexler lbwexler merged commit 352f144 into develop Aug 15, 2018
@amcclain amcclain deleted the dayFieldImprovements branch August 17, 2018 19:32
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.

2 participants