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

[EuiDatePicker] Handle invalid selected moment formats #7784

Merged
merged 6 commits into from
May 23, 2024

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented May 22, 2024

Summary

@shahzad31 reported this issue to us in Slack - passing an invalid moment() date to the selected prop causes a maximum update depth exceeded React error. We should be more gracefully handling and alerting consumers to this issue without just blanket crashing JS.

Before After
fix

QA

  • Go to https://eui.elastic.co/pr_7784/#/forms/date-picker
  • Confirm that the first example on the page shows an Invalid date value (with danger icon/coloring)
  • Confirm that clicking on the invalid date shows the calendar popover without crashing and allows the user to select a new valid date

General checklist

  • Revert [REVERT ME] commit
  • Browser QA
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
      - [ ] Checked in both light and dark modes
      - [ ] Checked in mobile
  • Docs site QA - N/A, bugfix
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
      - [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist - N/A

@cee-chen cee-chen added the bug label May 22, 2024
@cee-chen cee-chen self-assigned this May 22, 2024
@cee-chen cee-chen marked this pull request as ready for review May 22, 2024 21:12
@cee-chen cee-chen requested a review from a team as a code owner May 22, 2024 21:12
Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

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

🚢 🐈‍⬛ Nice fix! And the updated code works as expected 👍

@cee-chen cee-chen enabled auto-merge (squash) May 23, 2024 15:04
@kibanamachine
Copy link

Preview staging links for this PR:

@cee-chen cee-chen merged commit 7140a4e into elastic:main May 23, 2024
5 checks passed
@cee-chen cee-chen deleted the datepicker/invalid-date-fix branch May 23, 2024 15:31
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @cee-chen

tkajtoch added a commit to elastic/kibana that referenced this pull request May 29, 2024
`v94.5.1` ⏩ `v94.5.2`

_[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_

---

## [`v94.5.2`](https://github.com/elastic/eui/releases/v94.5.2)

**Bug fixes**

- Fixed `EuiDatePicker` to more gracefully handle incorrectly formatted
`selected` Moment dates, instead of simply crashing
([#7784](elastic/eui#7784))
- Fixed `EuiFlexGroup` and `EuiFlexItem` types to correctly accept
global attribute props and simplify type resolution when used with
`styled()`-like wrappers
([#7792](elastic/eui#7792))
rshen91 pushed a commit to rshen91/kibana that referenced this pull request May 30, 2024
`v94.5.1` ⏩ `v94.5.2`

_[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_

---

## [`v94.5.2`](https://github.com/elastic/eui/releases/v94.5.2)

**Bug fixes**

- Fixed `EuiDatePicker` to more gracefully handle incorrectly formatted
`selected` Moment dates, instead of simply crashing
([elastic#7784](elastic/eui#7784))
- Fixed `EuiFlexGroup` and `EuiFlexItem` types to correctly accept
global attribute props and simplify type resolution when used with
`styled()`-like wrappers
([elastic#7792](elastic/eui#7792))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants