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

Fix router lemmy link resolution #644

Merged
merged 7 commits into from
Sep 27, 2023

Conversation

boscojwho
Copy link
Contributor

@boscojwho boscojwho commented Sep 24, 2023

Checklist

Pull Request Information

About this Pull Request

Fixes issue where community links do nothing because navigation router didn't recognize them as a valid navigation route.

  • Previously, NavigationRouter was accepting raw data values from HandleLemmyLinkResolution, which is no longer supported.
  • We now ensure data values are mapped to their logical navigation route enum.

Screenshots and Videos

No UI changes.

Additional Context

Will follow up shortly with another PR that streamlines navigation routes a bit more.

  • Currently, this means the "Community Page" in Settings will open in in-app browser, since Settings router isn't configured to support Lemmy data values.

…mpting to append to `AnyNavigationPath`:

- Handler now wraps resolved value in the associated navigation path's route value type.
- Also, remove `AnyNavigationPath` conformance for `NavigationPath` because it no longer makes sense.
- Documentation.
@boscojwho boscojwho requested a review from a team as a code owner September 24, 2023 02:18
@boscojwho boscojwho requested review from JakeShirley and mormaer and removed request for a team September 24, 2023 02:18
Copy link
Contributor

@mormaer mormaer left a comment

Choose a reason for hiding this comment

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

Looks good, two minor comments/thoughts 🙃

Mlem/Navigation/Route/SettingsRoutes.swift Show resolved Hide resolved
Mlem/Navigation/Route/SettingsRoutes.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@mormaer mormaer left a comment

Choose a reason for hiding this comment

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

🚀

@boscojwho boscojwho merged commit 6348989 into dev Sep 27, 2023
4 checks passed
@boscojwho boscojwho deleted the bosco/fix-router-lemmy-link-resolution branch September 27, 2023 00:11
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