Skip to content

fix: do not update android dialog on each re-render #374

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

Merged
merged 1 commit into from
Jan 2, 2021

Conversation

vonovak
Copy link
Member

@vonovak vonovak commented Dec 23, 2020

Summary

replacement for #327, please see motivation there. To sum it up, android dialog's selected date is controlled from JS and while the dialog is open, it will update only if the date changed in JS.

edit: Previously we updated the value in the android dialog on each render, now we will update it only if the value has changed.

Closes #182
Closes #235
Closes #327

Test Plan

tested locally

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)
  • I have added automated tests, either in JS or e2e tests, as applicable

@vonovak vonovak requested a review from luancurti December 23, 2020 23:39
Copy link
Member

@luancurti luancurti left a comment

Choose a reason for hiding this comment

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

Sorry for the delay

Great Job! 🎉

@luancurti luancurti merged commit 16bff74 into master Jan 2, 2021
@luancurti luancurti deleted the fix/isolate-dialog-updates branch January 2, 2021 22:05
vonovak pushed a commit that referenced this pull request Jan 2, 2021
## [3.0.9](v3.0.8...v3.0.9) (2021-01-02)

### Bug Fixes

* do not update android dialog on each re-render ([#374](#374)) ([16bff74](16bff74))
@vonovak
Copy link
Member Author

vonovak commented Jan 2, 2021

🎉 This PR is included in version 3.0.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

@vonovak
Copy link
Member Author

vonovak commented Jan 5, 2021

@luancurti can you please check discord? I left a message in the datepicker channel, thank you! :)

@snios
Copy link

snios commented Jan 5, 2021

Just tested this new version that is supposed to not update on re-render. But it does not work for me atleast. I have a useEffect with setInterval that triggers a recalculation of time once each second. And the datepicker reverts my date while open and selecting, every time my value is recalculated. My picker is inside a modal.

@vonovak
Copy link
Member Author

vonovak commented Jan 5, 2021

@snios that is the expected behavior, JS is the source of truth for the picker:

To sum it up, android dialog's selected date is controlled from JS and while the dialog is open, it will update only if the date changed in JS

@snios
Copy link

snios commented Jan 5, 2021

@snios that is the expected behavior, JS is the source of truth for the picker:

To sum it up, android dialog's selected date is controlled from JS and while the dialog is open, it will update only if the date changed in JS

Hm. I dont change the js date that the picker uses as far as i know. The calculation that is done in useeffect is not connected with the date. The date picker is a separate component that houses its own logic.

EDIT
Ouch. I am on to something now. The modal component reinitializes each time my value is recalculated for some reason.
That in turn means my js value is reset in ctor each time as well. So you can disregard this comment.

@AmolFC
Copy link

AmolFC commented Apr 27, 2021

@snios We are also facing same problem in our code. Did you find solution for same?

@snios
Copy link

snios commented Apr 27, 2021

@snios We are also facing same problem in our code. Did you find solution for same?

Hi @AmoIFC. Issue for us was that in the parent view of the modal there was a setInterval that calculated som values and that made the modal component reinitialize (the constructor ran again). So in the view that has the setInterval i dont recalculate the values if modal is shown. Probably a bad solution but it works.

example.
useEffect(() => { if(modalVisible == false){ const tickerID = setInterval(() => { calculateTimers(); }, 1000); return () => clearInterval(tickerID); } });

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.

Value is being updated while picking Android: DateTimePicker keeps resetting to default value
5 participants