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

Hover over stops - faster #416

Merged
merged 3 commits into from
Nov 17, 2019
Merged

Conversation

zachzwy
Copy link
Contributor

@zachzwy zachzwy commented Nov 16, 2019

Fixes #272. Improves PR #362.

Use a different approach to create "hover over stops" effect. Instead of letting React rerender the whole routes (see PR #362 for detailed performance issue), it adds an "on-hover" className to the stop that being hovered. The "on-hover" class uses a pseudo class .on-hover::before to add stop title.

Proposed changes
When you hover over entries in the To & From Dropdowns, the stop/route on the map will update correspondingly. So you know exactly where the stop you're hovering is on the map.

Link to demo, if any
https://open-transit.herokuapp.com/route/M/direction/M____O_D00/fromStop/6996/toStop/7125

@zachzwy zachzwy requested a review from exxonvaldez November 16, 2019 17:29
@zachzwy zachzwy self-assigned this Nov 16, 2019
@@ -85,7 +85,7 @@ class MapStops extends Component {
// svg "v" shape rotated by the given rotation value.

icon = L.divIcon({
className: 'custom-icon', // this is needed to turn off the default icon styling (blank square)
className: `custom-icon id${stop.sid}`, // this is needed to turn off the default icon styling (blank square)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom-icon was just a dummy class (that's what the comment was about)... can you try removing it?

Copy link
Contributor

@exxonvaldez exxonvaldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice, hadn't occurred to me that using css would be this effective and compact. There is another PR that is rewriting the Select's to use a new class, will message you about it on Slack.

@@ -132,6 +133,23 @@ function ControlPanel(props) {
});
}

function handleMouseOver(node, title) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use jsdoc comments here to explain these methods since the names are fairly generic. Also could consider renaming the methods to be more specific (handleItemMouseOut, handleSelectClose).

@hathix
Copy link
Member

hathix commented Nov 17, 2019

Nice, so let's remember to close #362 when this one gets merged.

@hathix
Copy link
Member

hathix commented Nov 17, 2019

@zachzwy , go ahead and merge this whenever you can

@zachzwy zachzwy merged commit 6b4d56c into master Nov 17, 2019
@zachzwy zachzwy deleted the hightlight-stops-on-hovering-fast branch November 17, 2019 22:21
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.

Highlight Stops on hovering over items on To/From Dropdown
3 participants