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

onActiveStartDateChange doesn't get fired when clicking on January in the year view #458

Closed
bbycroft opened this issue Nov 24, 2020 · 3 comments
Labels
question Further information is requested

Comments

@bbycroft
Copy link

Steps to reproduce with https://codesandbox.io/s/intelligent-ganguly-6xlfy?file=/src/App.js:

  1. Click the year (e.g. November 2020) to go into the year view.
  2. (bug) Click on January, and notice that onActiveStartDateChange event doesn't get fired.
  3. Go back into year view, and try with any other month. In these cases, the event does get fired as expected.

This also applies when drilling down into the first year of a decade.

This is the correct behavior in the sense that the activeStartDate isn't actually changing when you drill down on the first month. Although it was quite surprising in our usage. I'd have preferred/expected the logic to be that the event gets fired iff the tuple (view, activeStartDate) is changed.

We can workaround this pretty easily by mapping onDrillDown to the same handler, at the (minimal) expense that we get duplicate calls when clicking on not-January months.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2021

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this issue will be closed in 14 days.

@github-actions github-actions bot added the stale label Oct 4, 2021
@github-actions
Copy link
Contributor

This issue was closed because it has been stalled for 14 days with no activity.

@wojtekmaj wojtekmaj reopened this Oct 25, 2021
@wojtekmaj wojtekmaj added question Further information is requested and removed stale labels Oct 25, 2021
@wojtekmaj
Copy link
Owner

I don't think this will be worked on (the logic for these callbacks is based on setting state and firing additional callback when state is changed, so this would require a hefty refactoring) but I do agree with you that this coming as a surprise should not have been the case. This should be better documented and I'm gonna update the docs to make note of that.

wojtekmaj added a commit that referenced this issue Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants