Skip to content

Conversation

zweatshirt
Copy link
Contributor

@zweatshirt zweatshirt commented Aug 27, 2025

Description

Jira ticket
Issue:

  • People could already technically delete anniversary values, they were just unable to do it by pressing enter.
    • This is true of any other date fields that would exist in the Contact Details modal (namely, the Birthday field).
  • People were also unable to save a valid date by pressing enter.
  • Both behaviors, being unable to save data on enter and being unable to clear data on enter, is inconsistent with other fields in the form.

Proposed solution:

  • Add a custom onKeyDown handler.

Other issues/thoughts:

  • There seems to be an issue with how DesktopDateField handles onBlur. This is consistent across pages and modals.
  • Since the handler is used twice, it can become a shared function, but since it is only used in two places I didn't go that route.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

Copy link
Contributor

Bundle sizes [mpdx-react]

Compared against 9aa4279

No significant changes found

@zweatshirt zweatshirt changed the title MPDX-8563: Allow anniversary delete (and input) on enter MPDX-8563: Allow anniversary delete and input on enter Aug 27, 2025
@zweatshirt zweatshirt force-pushed the MPDX-8563-delete-anniversary branch from 0e9c084 to 489fad2 Compare August 28, 2025 13:32
@zweatshirt zweatshirt marked this pull request as ready for review August 28, 2025 13:50
@zweatshirt zweatshirt requested review from dr-bizz and Copilot August 28, 2025 13:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds keyboard support for anniversary and birthday date fields, allowing users to save or clear dates by pressing Enter. This improves form consistency as other fields already support Enter key interactions.

Key changes:

  • Added handleOnDateEnter function to process Enter key events on date inputs
  • Enhanced date fields to handle both saving valid dates and clearing empty inputs on Enter
  • Added comprehensive test coverage for the new keyboard functionality

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
PersonShowMore.tsx Added Enter key handler for anniversary date field
PersonShowMore.test.tsx Added comprehensive test suite for anniversary date field interactions
PersonBirthday.tsx Added Enter key handler for birthday date field
PersonBirthday.test.tsx Added comprehensive test suite for birthday date field interactions

Comment on lines +103 to +115
const handleOnDateEnter = (event: React.KeyboardEvent<HTMLInputElement>) => {
if (event.key === 'Enter') {
const inputValue = (event.target as HTMLInputElement).value.trim();
if (!inputValue) {
handleDateChange(null);
} else {
const parsed = DateTime.fromFormat(inputValue, 'M/d/yyyy');
if (parsed.isValid) {
handleDateChange(parsed);
}
}
}
};
Copy link
Preview

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

This function is duplicated in both PersonShowMore and PersonBirthday components. Consider extracting it to a shared utility function to follow DRY principles and improve maintainability.

Copilot generated this review using guidance from repository custom instructions.

Comment on lines +62 to +74
const handleOnDateEnter = (event: React.KeyboardEvent<HTMLInputElement>) => {
if (event.key === 'Enter') {
const inputValue = (event.target as HTMLInputElement).value.trim();
if (!inputValue) {
handleDateChange(null);
} else {
const parsed = DateTime.fromFormat(inputValue, 'M/d/yyyy');
if (parsed.isValid) {
handleDateChange(parsed);
}
}
}
};
Copy link
Preview

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

This function is duplicated in both PersonShowMore and PersonBirthday components. Consider extracting it to a shared utility function to follow DRY principles and improve maintainability.

Copilot generated this review using guidance from repository custom instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll probably do this.

@dr-bizz dr-bizz requested review from canac and removed request for dr-bizz August 28, 2025 15:00
@canac
Copy link
Contributor

canac commented Aug 28, 2025

@zweatshirt Thanks so much for improving this! The way it worked before is definitely a bad user experience.

I have an idea for how to fix this at the source. The root cause seems to be that DesktopDateField.tsx only calls onChange in onBlur. I'd like for us to update onChange in DesktopDateField to try to parse the date that the user is starting to type. If It's an invalid date, do nothing because they probably haven't finished typing (e.g. "1/1/"). But if the input is empty or the date is a valid date, go ahead and call onChange. That way if they hit enter without tabbing away from the field, the form will have the date they just typed and be able to type it. Does that make sense?

I'm not saying you need to change it now, but DesktopTimeField has the same issue.

@zweatshirt
Copy link
Contributor Author

@zweatshirt Thanks so much for improving this! The way it worked before is definitely a bad user experience.

I have an idea for how to fix this at the source. The root cause seems to be that DesktopDateField.tsx only calls onChange in onBlur. I'd like for us to update onChange in DesktopDateField to try to parse the date that the user is starting to type. If It's an invalid date, do nothing because they probably haven't finished typing (e.g. "1/1/"). But if the input is empty or the date is a valid date, go ahead and call onChange. That way if they hit enter without tabbing away from the field, the form will have the date they just typed and be able to type it. Does that make sense?

I'm not saying you need to change it now, but DesktopTimeField has the same issue.

Thanks for the detailed response Caleb. I noticed the issue in DesktopDateField but tried to keep the PR in the scope of this Contact Details modal. I'll work on making that change now.

@canac
Copy link
Contributor

canac commented Aug 28, 2025

@zweatshirt Thanks for trying to keep the scope small! We definitely don't want to do more work than we need to. In this case though, I think it is worth solving the root issue. Then your fix will benefit any modal that has date picker inputs.

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