metrics-mvp
metrics-mvp copied to clipboard
render route on map on table row hover
Fixes #389
Proposed changes
Render route on map when hovering over route table. When no spider exists both directions are rendered. When spider exists only render directions already on the spider.
Currently on hover, the starting stop (closest stop to spider marker) of the route and stops are fully opaque whilst polylines are slightly less transparent (only when spider exists), and of course everything is slightly larger. I find that when the polylines are too transparent you sometimes see a mess of colors underneath making it harder to see but when it's too solid you can't see the stops. Thoughts?
Also the map shields of other routes currently stay on top of the hovered route (except the shield). Is this preferable?
Link to demo, if any
https://open-transit-hover-line.herokuapp.com/
Thanks for doing this. This seems like a good enhancement overall, and it is very responsive (which I know can be a challenge with react-leaflet). I haven't used React.memo before but this seems like a natural place to use it.
I wanted to start with some high level feedback before getting into a detailed review. On the technical side, I was wondering if we can avoid the use of setTimeout/clearTimeout. These kinds of global function calls are difficult to understand and debug. Also, pushing state from Dashboard down to its children is a bit of an antipattern that Redux was meant to solve. Can you use Redux state to communicate between components and avoid the use of setTimeout? The spider selection state is done using Redux, you should be able to add stuff alongside that.
On the UI design side, everything a matter of subjective opinion but my two cents: I'd like the see the hover effect done a little differently. On MapStops, we use a wider white polyline to provide some contrast against the map, and then a wider indigo polyline between the selected stops. So can we do the hover effect using a white or indigo outline? I'd also prefer the shield sizes and colored line widths to stay as they were. The width conveys the frequency of the line, so that information gets lost when the width is scaled up, especially if there is no spider selection. I'm hoping the white or indigo outline lets you keep the opacities for the colored polylines where there were originally, and no changes to the shields.
It seems natural that the hover effect should go both ways, so when you highlight a route segment, the route's row in the table is also highlighted (the light gray background in the table row, which might be a css effect).
Thanks for the detailed feedback.
My rationale behind the route only showing directions within range of the marker was that having a marker down already eliminated whole routes from the table making them unselectable, so the user would equate what is shown on the map to what remains on the table and vice versa. On the other hand clicking the route involves choosing either direction so if you still think it would be more natural to always show both directions then I don't disagree.
Regarding the timeout, removing it won't have a huge impact but it will be noticeable when moving through the routes consecutively at a somewhat fast rate.

My rationale behind the route only showing directions within range of the marker was that having a marker down already eliminated whole routes from the table making them unselectable, so the user would equate what is shown on the map to what remains on the table and vice versa. On the other hand clicking the route involves choosing either direction so if you still think it would be more natural to always show both directions then I don't disagree.
I'm not sure what part of my feedback you're referring to here? It looks the visibility of routes when a selection is made hasn't changed in this PR, and that is good. Aside from stylistic changes to how the route highlights are done, I was suggesting that when the user mouses over the map (triggering the tooltip), we also highlight the route's row in the table.
Regarding the timeout, removing it won't have a huge impact but it will be noticeable when moving through the routes consecutively at a somewhat fast rate.
For this particular feature, I think that is fine. Let's try it out.
My mistake, after I read the paragraph again it seems I misunderstood.
Removed timeout on heroku demo.
Hey Raymond,
We were all going crazy about your feature at the meeting just now. In the words of one of our members, it's "tight."
Couple pieces of feedback:

Terence was saying that the highlighting is very subtle now, making it hard to pick out the route. (Before it was a bit too much?) Perhaps something in between would be better.

In the "live" version of OpenTransit, each line is a different width based on how often the bus comes. I don't see that in this version of yours. Can you put back that feature?
I was suggesting that when the user mouses over the map (triggering the tooltip), we also highlight the route's row in the table.
Also a good suggestion from Terence. How hard would that be to add?
Only change that's a "must" is putting the variable line widths back. Other things can be put in separate issues so we can address them later.
I did already undo the changes to the scale, it just seems that the route stats didn't update beyond a point as you can see there are no scores on the route table. I didn't look into it as I had assumed it was from a conflict on the backend. If you go back to Feb 9 or earlier you will see there are stats and the lines reflect them.
As for highlighting the row it sounds possible.
The code looks pretty good, but I can't test it effectively. I think there's some problems with the latest GTFS feed for Muni, from looking at some of the newest PRs, so we have duplicate routes.
When I click a point on the map, I still see most or all routes in the table, where I'm expecting to see just the routes that are visible on the map. I don't see a code change in this PR that would cause that, or maybe I missed it.
It might be helpful to address the git conflicts, as this should simplify the diffs a bit. A lot of changes are due to the linter/prettier fixing things, which are now also fixed on master.