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: issue_94 #113

Closed
wants to merge 1 commit into from
Closed

Conversation

panzaverdeok
Copy link

Resolves #94

Copy link
Collaborator

@nmcharlton nmcharlton left a comment

Choose a reason for hiding this comment

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

@panzaverdeok I've requested some changes. Please ask if you want some help with them.

@@ -214,7 +214,7 @@ function Filter(props) {
format={getDateFormatLocale(true)}
value={dateStart}
onChange={handleDateStartChange}
maxDate={dateEnd}
//maxDate={dateEnd} Conrado Banegas comment 6/2021 issue #94
Copy link
Collaborator

Choose a reason for hiding this comment

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

A few comments:

  1. We shouldn't remove maxDate, but should set it to today's date if dateEnd is null or undefined.
  2. The same problem exists with minDate on the End Date KeyboardDatePicker (line 229). When dateStart is null or undefined, this should be set to a sensible minimum (e.g., Date(1900-01-01)).
  3. If code is not needed, don't comment it out – remove it. We can use git to see what has been removed and put it back in if necessary.
  4. There's no need to add comments like this to the code. After a while they get in the way of the code and cause the files to bloat. GitHub tracks line-by-line changes for us and we link commits to issues via PRs.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Nick.
It' ok, I'll try to develop another alternativa to solve the issue.
Thanks.

@panzaverdeok panzaverdeok deleted the issue_94 branch July 11, 2021 15:35
@panzaverdeok panzaverdeok restored the issue_94 branch July 11, 2021 18:04
@panzaverdeok panzaverdeok deleted the issue_94 branch July 11, 2021 18:04
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.

Filter date selection: Cannot move forward
2 participants