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

Apple Annotation parsing #287

Merged
merged 20 commits into from
Oct 29, 2024
Merged

Apple Annotation parsing #287

merged 20 commits into from
Oct 29, 2024

Conversation

Archdoog
Copy link
Collaborator

@Archdoog Archdoog commented Oct 7, 2024

  • Removes MaxSpeed and associated types from common (no longer needed - we're just passing a JSON blob through to the platform code).
  • Adds AnnotationPublishing and AnnotationPublisher for piping navigation state into parsed typed annotation publishers.
  • Links speed limit view to annotation publisher @Published var speedLimit

#271 for iOS

@Archdoog Archdoog added the iOS label Oct 27, 2024
@Archdoog Archdoog self-assigned this Oct 27, 2024
@Archdoog Archdoog added the UI/UX Related to the UI/UX (on any platform) label Oct 27, 2024
@Archdoog Archdoog marked this pull request as ready for review October 27, 2024 17:24
@Archdoog Archdoog changed the title WIP: Apple Annotation parsing Apple Annotation parsing Oct 27, 2024
@ianthetechie
Copy link
Contributor

FYI @Patrick-Kladek @engali94. I know that you had a PR that I think is semi-blocked on this (#293). Feedback welcome.

@engali94
Copy link
Contributor

@ianthetechie I believe the new implementation by @Archdoog is more flexible and serve the purpose well.
I am going to close my pr in favor of this implementation. Do we have something similar for Android?

@ianthetechie
Copy link
Contributor

Sounds good, @engali94! Thanks for the review!

Re: Android, we don't have a PR yet, but I think you're more than welcome to tackle it.

I think that the closest thing to this which we have to Codable in Kotlin is the kotlinx.serialization library (but don't take that as absolute truth; I'm a much stronger Swift developer than Kotlin).

@Archdoog anything you want to add before the HudHud team takes a crack at it?

Copy link
Contributor

@ianthetechie ianthetechie left a comment

Choose a reason for hiding this comment

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

Awesome! This is really close now!

One thing besides the comments is that I'm not seeing any guide changes. I think we should add documentation of how the custom annotation deserialization works (on iOS, with a stub for Android).

We should also document how to configure the speed limit display.

@Archdoog
Copy link
Collaborator Author

Archdoog commented Oct 29, 2024

Sounds good, @engali94! Thanks for the review!

Re: Android, we don't have a PR yet, but I think you're more than welcome to tackle it.

I think that the closest thing to this which we have to Codable in Kotlin is the kotlinx.serialization library (but don't take that as absolute truth; I'm a much stronger Swift developer than Kotlin).

@Archdoog anything you want to add before the HudHud team takes a crack at it?

I'll write up the Android ticket(s) and follow up iOS ticket for traffic congestion ticket once we merge this so I can link and reference to things.

@ianthetechie ianthetechie merged commit 14b41be into main Oct 29, 2024
14 checks passed
@ianthetechie ianthetechie deleted the feat/apple/annotation-parsing branch October 29, 2024 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS UI/UX Related to the UI/UX (on any platform)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants