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] onSelect is fired when navigating between months #4270

Closed
smhutch opened this issue Nov 16, 2020 · 6 comments
Closed

[EuiDatePicker] onSelect is fired when navigating between months #4270

smhutch opened this issue Nov 16, 2020 · 6 comments
Labels

Comments

@smhutch
Copy link
Contributor

smhutch commented Nov 16, 2020

About

onSelect is fired twice when navigating between months In EuiDatePicker. My feeling is that it should not fire at all, until the user selects a date. Or, if changing month is considered selection, it should only fire once.

The same behaviour exists for onChange (though I changed to using onSelect, since it seems reasonable for onChange to be called when changing month).

Steps to replicate

Sandbox showing the issue. To see the issue, open the console, and check for calls to onSelect when:

  1. Clicking on a date
  2. Clicking on the arrows to change month

Note

Setting adjustDateOnChange={false} prevents this behavior.

@thompsongl
Copy link
Contributor

Thanks as always for the repro link, @smhutch

I think a single onSelect call is ok for month navigation, but I agree that the second is not needed.

@smhutch
Copy link
Contributor Author

smhutch commented Nov 18, 2020

Thanks @thompsongl, repo links feel especially important when I'm using some complex combinations of props. I usually make them to confirm if it's an issue in eui or just with my code before opening an issue 😄.

I think a single onSelect call is ok for month navigation

I'm not sure if that makes sense, at least it would be problematic for the way I'm trying to use the component currently. In my case, I have an inline EuiDatePicker, and I'm performing a bunch of additional state updates when the user chooses a date from the available options we display. The problem I see with firing onSelect, is that it gives me no way to know when a user actually selected a date (without relying on some additional logic at my end, as shown below). Am I overlooking something?

<EuiDatePicker
    // only show certain dates (derived from some other application state) 
    includeDates={
      data
        ? Object.keys(data).map((isoDate) => moment(isoDate).tz(timezone))
        : []
    }
    onSelect={(momentDate, event) => {
      // Hack alert. onSelect is fired when changing month.
      // event is only defined when actually selecting a date.
      if (event && momentDate && data) {
        const isoDate = momentDate.format(API_DATE_FORMAT);
        dispatch({
          type: 'setDate',
          availability: data[isoDate],
          date: isoDate,
        });
        onProceed();
      }
    }}
    inline
  />

I guess what I'm looking for is an event which fires when a date is explicitly clicked on (which triggers the picker to close, in this new repo)

@thompsongl
Copy link
Contributor

Does setting adjustDateOnChange={false} help for your case?
The prop documentation isn't great right now as EuiDatePicker is largely a wrapper around react-datepicker. We have plans (first part of 2021) to look at aligning the component more with EUI, so I think that would be the first step before any real behavioral changes are made.

@smhutch
Copy link
Contributor Author

smhutch commented Nov 20, 2020

@thompsong Thanks! I can confirm that setting adjustDateOnChange={false} works well for me. With that set to false, onSelect is only fired when explicitly selecting a date 👍

I'll keep this ticket open, but edit the description to reflect this.

@thompsongl
Copy link
Contributor

This will be resolved once #4243 merges

@thompsongl
Copy link
Contributor

Resolved by #4243

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

No branches or pull requests

2 participants