Skip to content

fix: display snaps on first render if zoom within threshold#112

Merged
jessicamcinchak merged 1 commit intomainfrom
jess/snaps-first-render
Feb 4, 2022
Merged

fix: display snaps on first render if zoom within threshold#112
jessicamcinchak merged 1 commit intomainfrom
jess/snaps-first-render

Conversation

@jessicamcinchak
Copy link
Copy Markdown
Member

@jessicamcinchak jessicamcinchak commented Feb 4, 2022

Previosly, we'd only render snaps after a map move (zoom, pan, etc) - this change makes snaps visible on the first render before any interactions if other conditions are met (drawMode is enabled, zoom is greater than or equal to 20)

@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 4, 2022

✔️ Deploy Preview for oslmap ready!

🔨 Explore the source changes: 5cc32de

🔍 Inspect the deploy log: https://app.netlify.com/sites/oslmap/deploys/61fd19ba3736210008af3e58

😎 Browse the preview: https://deploy-preview-112--oslmap.netlify.app/

@jessicamcinchak jessicamcinchak requested a review from a team February 4, 2022 12:19
@jessicamcinchak
Copy link
Copy Markdown
Member Author

Copy link
Copy Markdown
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Great 👍

Comment thread src/my-map.ts
Comment on lines +390 to +391
// define zoom threshold for showing snaps (not @property yet because computationally expensive!)
const snapsZoom: number = 20;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that making this accessible would be a bad idea!

@jessicamcinchak jessicamcinchak merged commit 3757159 into main Feb 4, 2022
@jessicamcinchak jessicamcinchak deleted the jess/snaps-first-render branch February 4, 2022 14:36
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