Transitive styles #366
Transitive styles #366
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #366 +/- ##
========================================
- Coverage 9.78% 8.14% -1.64%
========================================
Files 115 195 +80
Lines 4078 7340 +3262
Branches 1077 1843 +766
========================================
+ Hits 399 598 +199
- Misses 3191 5805 +2614
- Partials 488 937 +449
Continue to review full report at Codecov.
|
| !isEqual(nextProps.activeSearchVisibleItinerary, this.props.activeSearchVisibleItinerary) || | ||
| !isEqual(nextProps.activeSearchResponseOtp, this.props.activeSearchResponseOtp) |
There was a problem hiding this comment.
This could end up being quite an expensive operation because of the deep comparisons done to look at each of these items. It'd be much better to have a simple equality check or at least not check so many items.
There was a problem hiding this comment.
It seems there are two overall goals of this PR
- Use the
getTransitiveRouteLabelfunction from theComponentContext - Avoid as many rerenders as possible
However, as noted in the comments, this PR kind of has stuff all over the place and the method in which the shouldComponentUpdate is calculated could actually be a very expensive operation if every part of two objects need to be analyzed. Therefore, I propose an alternate way of doing things that consolidates the code and uses selectors to avoid recomputations. See #370.
Use selector to compute transitive data
|
Ready for review again... |
There was a problem hiding this comment.
Contingent upon addressing @landonreed's comments and also on merging in #379.
Avoid unnecessary rerenders
Co-authored-by: Evan Siroky <[email protected]>
|
🎉 This PR is included in version 3.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR modifies code to use new styling props introduced in
@opentripplanner/core-utils/transitive-overlaysv 1.0.7 (see opentripplanner/otp-ui#246):example-config.yml#map.getTransitiveRouteLabelfunction to render a route label is introduced in theComponentContext,TransitiveCanvasOverlayis updated accordingly.EDIT: Example configuration for testing: see https://github.com/ibi-group/configurations/pull/141.
EDIT: This PR should fix #374.