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

Change webcam icon to point in the direction the webcam is facing #21

Merged
merged 9 commits into from
Nov 25, 2023

Conversation

wmbr
Copy link
Contributor

@wmbr wmbr commented Nov 14, 2023

I think it's very useful to see on the map already what direction a webcam is facing, and this data is often available.
To this end I changed the icon to be more directional (svg taken from https://de.wikipedia.org/wiki/Datei:Video_camera_icon_svg.svg) and rotated it appropriately.

@wvanderp
Copy link
Owner

wvanderp commented Nov 15, 2023

Hello,

Thanks for creating this merge request. The idea is good, and I would like to add it. I have a few requests before we can merge the changes.

First, the good. I like the extra information it brings to the map. I also like the way it uses multiple tags to determine the direction.

Here is a list of things that need to be fixed before I can merge it.

  • Remove the brownish outline on the icon
  • fix the eslint error
  • fix the typescript error by installing @types/leaflet-rotatedmarker
  • add thanks for the new icon to the read me file
  • resolve the code comments

Thanks again for supplying this PR, and I look forward to the fixes and merging it.

src/types/webcam.ts Outdated Show resolved Hide resolved
utils/collect.ts Outdated Show resolved Hide resolved
utils/collect.ts Outdated Show resolved Hide resolved
src/components/pages/MapView.tsx Outdated Show resolved Hide resolved
src/components/pages/MapView.tsx Outdated Show resolved Hide resolved
@wmbr wmbr requested a review from wvanderp November 19, 2023 10:18
Copy link
Owner

@wvanderp wvanderp left a comment

Choose a reason for hiding this comment

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

I hate to do this to you. And I'm always displeased when somebody has another round of comments for me.

I've updated all the notes and checked if they are still valid. The effort is appreciated.

src/components/pages/MapView.tsx Outdated Show resolved Hide resolved
src/static/camera.svg Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@wvanderp wvanderp merged commit c535992 into wvanderp:master Nov 25, 2023
6 checks passed
@wvanderp
Copy link
Owner

First of all, thank you for making this.

Sorry for the mess I've made of the PR. I wanted to rebase and had some git mishaps along the way.

I also put some finishing touches on the code myself. You can find them in 49bb94e.

Thank you for having this idea and taking the time to implement it.

@wmbr wmbr deleted the webcam-direction branch November 25, 2023 20:47
@wmbr
Copy link
Contributor Author

wmbr commented Nov 25, 2023

Very nice. I love the new "grouping" feature on zoom out. Thank for starting this project.

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