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.

Sign up for GitHub

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

Jump to bottom

Don't show add popup when click on map #86

Merged
merged 8 commits into from
Jul 19, 2024
Merged

Don't show add popup when click on map #86

merged 8 commits into from
Jul 19, 2024

Conversation

nielsbom
Copy link
Collaborator

@nielsbom nielsbom commented Jul 5, 2024

Fixes #68

@nielsbom nielsbom requested a review from jokroese July 5, 2024 14:32
@netlify Netlify
Copy link

netlify bot commented Jul 5, 2024 •
edited
Loading

✅ Deploy Preview for qtmmirror ready!

Name Link
🔨 Latest commit 492194f
🔍 Latest deploy log https://app.netlify.com/sites/qtmmirror/deploys/669abbb517087f00081b79f9
😎 Deploy Preview https://deploy-preview-86--qtmmirror.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 configuration.

jokroese
jokroese reviewed Jul 5, 2024
View reviewed changes
src/lib/Map.svelte Outdated Show resolved Hide resolved
jokroese
jokroese requested changes Jul 5, 2024
View reviewed changes
Copy link
Member

@jokroese jokroese left a comment

Choose a reason for hiding this comment

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

Two small changes:

  • the input on line 129 is not a MapMouseEvent. I don't know what it is though without thinking about it a bit! We should either remove this type or find the correct one
  • on line 18, 'addOverlayVisible' is defined but no longer used

Otherwise, good to go!

@nielsbom
Copy link
Collaborator Author

nielsbom commented Jul 7, 2024

'addOverlayVisible' is defined but no longer used

I'd like to run npm run lint and have it discovered but for some reason it gives no feedback and (shell) exit code 1 so no error. Do you have any idea why?

@jokroese
Copy link
Member

jokroese commented Jul 8, 2024

Before I think too hard, can you run npm install to check you have the current version of prettier and eslint? I upgraded them in a recent push and not having the latest versions would cause a similar issue to what you have.

@jokroese
Copy link
Member

How's this going @nielsbom?

jokroese added 6 commits July 19, 2024 20:46
@jokroese
remove unused variable
cf0b030
@jokroese
use correct type for event object
bc52800
@jokroese
run lint and format
aec0861
@jokroese
Merge branch 'main' into no_add_on_map_click
76ede87
@jokroese
avoid offset type error
ac9cc09
@jokroese
bump dependencies
b280206 
there's some issue with the lint during the ci github workflow, not locally. I'll bumped dependencies because a) good and b) might shift an issue with the cache or something else
@jokroese
Copy link
Member

the ci github workflow is having issues with installing prettier, suggesting it's an issue with npm 😅

@jokroese
Copy link
Member

It does indeed seem to be with npm: npm/cli#7672.

@jokroese
use stable version of node
492194f 
avoid issues with npm
@jokroese jokroese merged commit 485b9eb into main Jul 19, 2024
7 checks passed
@jokroese jokroese deleted the no_add_on_map_click branch July 19, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jokroese jokroese jokroese approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Revise 'add pin click' functionality to avoid overlay opening on map clicks
2 participants
@nielsbom @jokroese