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

feat: Climb/Route coordinates #1092

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

alexoj46
Copy link

@alexoj46 alexoj46 commented Feb 9, 2024


name: feat: Climb/Route coordinates


What type of PR is this?(check all applicable)

  • refactor
  • feature
  • bug fix
  • documentation
  • optimization
  • other

Description

Implemented FrontEnd solution to Issue #1078 to add climb/route coordinates to climb pages.

Related Issues

Issue #1078

What this PR achieves

Added FrontEnd component to climb/route page to display route coordinates. The component will also link to Google Maps, and will add a tick to the map on the website.

Screenshots, recordings

Screenshot 2024-02-08 at 7 45 40 PM

Notes

If coordinates are not present in metadata for that climb, the lat, lng display will still appear, with NA, NA as the value (as shown in the photo).

Also added my contributor information.

Copy link

vercel bot commented Feb 9, 2024

@alexoj46 is attempting to deploy a commit to the openbeta-dev Team on Vercel.

A member of the Team first needs to authorize it.

@btmccord
Copy link
Contributor

Rather than showing NA, we should inherit the parents coordinates.

@musoke
Copy link
Contributor

musoke commented Feb 11, 2024 via email

@btmccord
Copy link
Contributor

See #1057 for discussion of why inheriting parent coordinates may not be the best approach.

That makes sense for areas, however if we have a coordinate for an area we should show it on the route page as a very high percentage of the time they will be that same (for our purposes as climbers).

Route coordinates are only really usefully for a handful of areas that are very low density.

What about indicating that the coordinates displayed are that of the area?

@musoke
Copy link
Contributor

musoke commented Feb 11, 2024 via email

@vnugent
Copy link
Contributor

vnugent commented Feb 11, 2024

I've just looked at the backend code. When creating a new route, the backend populates the climb's coordinate pair with that of the parent. We can hash out more detail in #1057. For this PR we can just deal with the visual. If the climb object doesn't have coordinates, it's not easy to fetch the parent's nor it is worth it to deal with in the frontend (I think the backend is a better place for it). Can we just not show it if that's the case?

What about adding some wording to indicate the coordinates are inherited form the parent? Something like 34.18150, -85.81285 (Inherited)?


Screenshot 2024-02-11 at 10 40 40 AM

Copy link

vercel bot commented Feb 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
open-tacos ✅ Ready (Inspect) Visit Preview Feb 11, 2024 6:58pm

@btmccord
Copy link
Contributor

When creating a new route, the backend populates the climb's coordinate pair with that of the parent.

If this is the case I would think it's not possible to tell if it's inherited or not...?

I guess that right now they are all inherited since we have now editing functionality for climb coordinates currently?

@vnugent
Copy link
Contributor

vnugent commented Feb 12, 2024

I guess that right now they are all inherited since we have now editing functionality for climb coordinates currently?

Right now they all inherited, but we can't edit climb coordinates at the moment.

Comment on lines +149 to +150
34.18008,
-85.81680
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you swap the order? Coordinate pair is expressed as longitude/lat (x,y) in geojson

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.

4 participants