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

Bug: events break when changing hoursInDisplay #133

Merged
merged 2 commits into from
Sep 17, 2021

Conversation

pdpino
Copy link
Collaborator

@pdpino pdpino commented Sep 8, 2021

Fixes #131

When hoursInDisplay change on the fly, the events' style (position) must be recalculated, and the NowLine must be repositioned

  • The main change in Events.js: the processEvents() method is now also memoized by hoursInDisplay
    • I refactored some methods into functions --> so it is more clear on which props do they depend, when they use this.props or this.state, etc --> to avoid bugs like this one (regarding memoization)
  • The change in NowLine.js: the line is updated in componentDidUpdate

@pdpino pdpino changed the title Bug: change hours display Bug: events break when changing hoursInDisplay Sep 8, 2021
@hoangnm
Copy link
Owner

hoangnm commented Sep 17, 2021

I agree that it's cleaner. LGTM. Thanks, @pdpino !

@hoangnm hoangnm merged commit 0c8ed2f into hoangnm:master Sep 17, 2021
@pdpino pdpino deleted the bug/change-hours-display branch October 16, 2021 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug when switching hoursInDisplay
2 participants