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

Indicate the side of road for pavement width #506

Merged
merged 3 commits into from
May 17, 2024
Merged

Conversation

dabreegster
Copy link
Contributor

Needs the new pmtiles, of course

@@ -82,27 +108,33 @@
visibility: show ? "visible" : "none",
}}
>
<Popup let:props>
<Popup let:props openOn="hover">
{#if props.left_average}
Copy link
Contributor

Choose a reason for hiding this comment

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

My overzealous DRY instincts are kicking in, is it overkill to turn this into its own component rather than having mostly the same code for both left and right?

Copy link
Contributor

@Pete-Y-CS Pete-Y-CS left a comment

Choose a reason for hiding this comment

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

Fine and certainly makes things better but a small comment about maybe turning the left/right parts of the popup into a component to be more DRY. Not a blocker but a maybe-nice-to-have

@Pete-Y-CS
Copy link
Contributor

new pmtiles uploaded

@dabreegster
Copy link
Contributor Author

Refactored the popups, but noticed the directions are wrong. There are 8 directions, 360/8 is 45, but north shouldn't be 0 to 45, it should be split to handle 360 - 45/2, and so on. Does that make sense? It's late, can pick up next week

@dabreegster dabreegster merged commit 47322d1 into main May 17, 2024
2 checks passed
@dabreegster dabreegster deleted the pavement_direction branch May 17, 2024 09:50
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