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

fix: Ensure snap points always load #193

Merged
merged 6 commits into from
Aug 26, 2022

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Aug 25, 2022

Addresses https://trello.com/c/NZrvqg7d/2055-get-snap-points-on-map-to-always-load

Currently, points are automatically generated on the assumption that vector tiles will be loaded. On slower connections, this means that snap points either do not display or do not fully display. This can be mocked in the browser by throttling your connection speed.

This PR fixes this by waiting for the map loadend event before triggering the generation of the snap points. Originally I was listening for the vector source's tileloadend event but this fires for each tile load and then regenerates snap points which I imagine is both computationally expensive and could lead to inconsistent results (e.g. all snap points loading in one tile at a time).

Also - some basic OL tests! Couldn't quite get around a few event dispatch issues so I'm manually calling them currently. This would be great to try and remove - possibly something to tackle when/if we add Cypress to the stack.

@netlify
Copy link

netlify bot commented Aug 25, 2022

Deploy Preview for oslmap ready!

Name Link
🔨 Latest commit a9bf8f0
🔍 Latest deploy log https://app.netlify.com/sites/oslmap/deploys/6308d5dcfc20160008eed209
😎 Deploy Preview https://deploy-preview-193--oslmap.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@DafyddLlyr
Copy link
Contributor Author

Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

This works great for me - snaps load a lot more consistently on a slow connection!

Thanks for tests too - there's some great foundational stuff there in addition to the snap-specific logic. A few minor comments but no blockers for me!

@DafyddLlyr DafyddLlyr merged commit 58a6dfb into main Aug 26, 2022
@DafyddLlyr DafyddLlyr deleted the dp/add-snap-points-after-map-load branch August 26, 2022 14: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.

2 participants