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

Bug - DatePicker - onBlur is not called on all field changes #8779

Closed
Chip-L opened this issue Mar 6, 2023 · 4 comments · Fixed by #9373
Closed

Bug - DatePicker - onBlur is not called on all field changes #8779

Chip-L opened this issue Mar 6, 2023 · 4 comments · Fixed by #9373
Assignees
Labels
Milestone

Comments

@Chip-L
Copy link

Chip-L commented Mar 6, 2023

Describe the problem
onBlur is only called if a value is entered in the text input. It is important to fire the onBlur event because this also triggers the validation (we can’t catch an empty field that is required) and updates any error messages.

How do you reproduce the problem?

  1. Open the CodeSandbox for the DatePicker component: https://o4yftx.csb.app/
  2. Click in the date text field and press Tab (triggers onBlur)
  3. Click on the Calendar Icon
  4. Observe in the Console.log, onBlur message didn’t appear (this is expected because no change)
  5. Select a date on the calendar.
  6. Observe in the Console.log, onChange message appeared, but not onBlur
  7. Click in the date text field and press Tab again
  8. Observe the console shows onBlur message (this is expected and correct)
  9. Delete the date in the text field and press Tab
  10. Observe the console now shows the onChange but not onBlur

Expected behavior
It is expected that onBlur (and any other validator) will be called whenever a value is changed, either by the calendar or by inputting the text.

Is this issue blocking you?
Our workaround is to use a copy of the DatePicker component with the following changes:
• For the change text onBlur is to remove the if(pristine) check.
• For the calendar is change the <Popover>’s props to include onHide={() => onInputBlur(null)}.

Screenshots
N/A

What is your environment?

  • OS: Windows 10
  • Browser: Chrome, Firefox
  • Version: Latest

What is your product and what release date are you targeting?
PMWeb, an insurance actuator product. We do continuous releases.

Any other information?
I believe this issue is a result of the fix for bug #6822 (released in v4.192.4). It looks to me like the value of pristine was changed to mean "is empty" from "is unchanged". The ability to change from a valid date to an empty date without it displaying an error is a valid use case. However, that doesn’t mean we can skip the validation triggered by onBlur. It is also a valid use case to not allow an empty date field.

@stale
Copy link

stale bot commented May 9, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@tiagojufr
Copy link

This issue was reported in v4, from what I understand this was only fixed in v5. Can the fix be backported to v4?

@hugoribeiromsglife
Copy link

Updated the sandbox above to version @patternfly/[email protected] that should have this fixed but it is still not working as expected. onBlur is not being triggered when we click on a date in the calendar popover. Will you reopen this issue or should I create a new one?

@thatblindgeye
Copy link
Contributor

thatblindgeye commented Aug 30, 2023

@hugoribeiromsglife currently the onBlur prop is meant only for when the text input loses focus. From the sounds of it, the onChange prop may be more applicable for triggering a callback upon selecting a date in the Calendar popover. Internally onChange is called as part of a date being clicked:

const onDateClick = (_event: React.MouseEvent<HTMLButtonElement, MouseEvent>, newValueDate: Date) => {
const newValue = dateFormat(newValueDate);
setValue(newValue);
setValueDate(newValueDate);
setError(newValueDate);
setPopoverOpen(false);
onChange(null, newValue, new Date(newValueDate));
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants