pacificclimate / weather-anomaly-tool

0 stars 0 forks source link

Fix markers #110

Closed rod-glover closed 1 month ago

rod-glover commented 1 month ago

Resolves #108 Resolves #109

This is an important fix; without it data is displayed inconsistently and wrongly.

Erroneous behaviour was that going back a few months (one at a time, with the -1) control and then moving forward again showed inconsistent (not the same!) markers.

Fix: Now consistent. Reviewer should verify; I can provide support for that.

rod-glover commented 1 month ago

Demo

Currently deployed production version behaves correctly, for comparison.

if you wish to see incorrect behaviour, clone the repo and check out commit d5c98752 / "Merge pull request #107 from pacificclimate/misc-fixes", then start app locally

rod-glover commented 1 month ago

Note: I think the upgrade to React 18 may have introduced some different, more "careful" behaviour by React with keys on rendered components. Which meant a shortcoming suddenly became visible a little while ago in my work. This is as well to know for other projects.

corviday commented 1 month ago

So the demo and the production version both behave correctly? The error was introduced in a version that has not yet been deployed to production, and thankfully caught early?

rod-glover commented 1 month ago

So the demo and the production version both behave correctly? The error was introduced in a version that has not yet been deployed to production, and thankfully caught early?

Yes, thank goodness. Earlier, I thought not.

corviday commented 1 month ago

Took me a while to understand the actual problem, but here's an example from SE BC, running the d5c9875 version:

Start at April 2024: april1

Click backwards to March 2024: march1

Click backwards ti February 2024: feb1

Now click forward to return to March 2024, but notice the colour coding of the stations is different than when we previously viewed March 2024. Perhaps it is retaining the February 2024 coloration?: mar2

Click forward to return to April 2024, which again does not match the previous viewing of this data: apr2

I have confirmed this behavior does not occur in the above-linked demo.

rod-glover commented 1 month ago

Excellent, thank you both!