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

performance(calendar): Prevent unnecessary update on day component #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abdallaayman21
Copy link
Collaborator

@abdallaayman21 abdallaayman21 commented Mar 12, 2024

Calendars where taking too long to update after a day is pressed, this was due unnecessary re-renders on the day component.

The re-renders are triggered due to comparing the onPress functions as well as theme.

Comparing the onPress function will alway return false if the onPress function uses useCallback or any similar hook that will update the fuction as it changes it's address in the memory therefore triggering a re-render.

However, comparing onPress or onLongPress is crucial for the ExpandableCalendar component as it's onPress function has multiple jobs like closing the calendar on day select if it was true. If we stopped comparing the onPress or onLongPress functions the ExpandableCalendar closeOnDayPress won't work as expected.

TODO:

  • Find a way to make the closeOnDayPress work without having to re-render all of the day components.
  • Make sure all test cases pass.

@abdallaayman21
Copy link
Collaborator Author

I will be using the fix/calendars-take-too-long-to-update branch in our code until we merge it and do all the required fixes to make the test cases pass

@abdallaayman21
Copy link
Collaborator Author

abdallaayman21 commented Mar 12, 2024

We could consider this as a temporary fix for us, but I think it we could make it better, as I think comparing the onPress and onLongPress is important but needs to be optimized

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 this pull request may close these issues.

1 participant