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

SuperDatePicker gets stuck in "update" mode. #4025

Closed
matt-moses opened this issue Sep 10, 2020 · 1 comment · Fixed by #4029
Closed

SuperDatePicker gets stuck in "update" mode. #4025

matt-moses opened this issue Sep 10, 2020 · 1 comment · Fixed by #4029

Comments

@matt-moses
Copy link

matt-moses commented Sep 10, 2020

Summary: SuperDatePicker gets stuck in "update" mode. By update mode I mean the following:

image

Scope:
I have observed this in my own project and in my test and prod environments but the issue described herein can also be reproduced by using one of the time pickers here: https://elastic.github.io/eui/#/forms/super-date-picker
In the screenshots I'm including, I'll use the very first time picker on that page if anyone wants to reproduce it the issue there.

This issue manifests itself if the user starts to edit the time but ends up making no changes. At face value this seems like a limited use case (and maybe it is) but there is another issue (#4026) with the SuperDatePicker that actually makes this happen more often for me. Due to #4026 because the date picker (1) doesn't restore the correct relative date to the GUI BUT (2) because it still has the correct underlying start/end date this issue becomes a more common issue. E.g. If I go back into edit mode of the time picker I can end up "editing" the start/end date without making any actual date changes.

Steps to reproduce:

  1. Go to https://elastic.github.io/eui/#/forms/super-date-picker

  2. Click on the "start" date box to edit the start date
    image

  3. Change the "30" minutes to "31" but don't hit "update" yet.
    image

  4. Observe the time picker is in "update mode"

  5. Switch the "31" back to "30"

  6. Observe the time picker is in "update mode" still
    image

  7. Click "Update"

  8. Observe button goes to "Updating..." and back to "Update". The time picker is now stuck in update mode until you go to change the time.
    image

I believe the root cause is in the getDerivedStateFromProps of the EuiSuperDatePicker component. https://github.com/elastic/eui/blob/master/src/components/date_picker/super_date_picker/super_date_picker.tsx#L199

I believe it is the result of the optimization the function has. (A very good optimization I would add). I believe a valid fix would be to make the optimization check something like the following (pseudo code):

      nextProps.start !== prevState.prevProps.start ||
      nextProps.end !== prevState.prevProps.end ||
      hasChanged
    ) {

My point is this: The hasChanged state gets set to true in the scenario above but because the start and end date haven't truly changed hasChanged is never set back to false; thus, leaving it in "update mode".

I'm a really big fan of EUI and really appreciate the effort that the Elastic team has put into this. So thanks to everyone!

@chandlerprall
Copy link
Contributor

Thank you for the great descriptions of both issues, and tracking down the relevant code location!

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 a pull request may close this issue.

2 participants