fix(router): update browser URL when navigating to drink detail on web - #289
Merged
Conversation
Contributor
LCOV of commit
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates festival-scoped routing to ensure drink-detail navigation updates the browser URL on Flutter web, and standardizes drink deep-link format to include a category path segment for better SEO/App Links alignment.
Changes:
- Added
navigateToRoute()to usego()on web (URL updates) andpush()on non-web (preserves native back stack). - Updated drink-detail path building to
/\:festivalId/drink/\:category/\:idand migrated navigation call sites accordingly. - Updated router/widget/E2E tests to assert the new 4-segment drink URL format.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/screens_test.dart | Uses a const Festival in setup to match immutable model usage. |
| test/router_test.dart | Adds widget coverage asserting drink-detail URLs include festivalId/drink/category/id. |
| test-e2e/routing.spec.ts | Updates Playwright routing expectations to the new drink deep-link format. |
| mise.lock | Updates the pinned Node.js version used by mise-managed tooling. |
| lib/widgets/drink_list_section.dart | Routes drink card taps through navigateToRoute() for web URL correctness. |
| lib/utils/navigation_helpers.dart | Introduces navigateToRoute() and updates drink-detail path builder to include category. |
| lib/screens/drinks_screen.dart | Uses navigateToRoute() for drink-detail navigation; minor const cleanup. |
| lib/screens/drink_detail_screen.dart | Routes style/brewery navigations through navigateToRoute(). |
| lib/main.dart | Routes favorites-list drink taps through navigateToRoute(). |
Contributor
🚀 Cloudflare Pages PreviewYour preview deployment is ready! Preview URL: https://claude-fix-drink-deep-links.staging-cambeerfestival.pages.dev This preview will be automatically updated when you push new commits to this PR. |
Owner
Author
|
@copilot review |
Contributor
Reviewed the current PR state; no additional changes were needed from this comment request. Current reviewed commit: |
GoRouter's push() from within a ShellRoute does not update currentConfiguration.uri on web, so the URL stayed stale after tapping a drink card. Fix by introducing navigateToRoute(), which calls context.go() on web (URL-updating) and context.push() on mobile (stack-preserving). Drink detail route now includes category segment (/:festivalId/drink/:category/:id) for SEO and App Links alignment. buildDrinkDetailPath gains a required category param; all call sites updated (drinks_screen, drink_detail_screen, main, drink_list_section). Share URLs now include the category segment so shared links resolve correctly to the new route format. New router_test covers that the URL contains all four segments after navigation. E2e routing spec updated to the four-segment URL format. https://claude.ai/code/session_01FqdVsDvtHXyWSszZA8tvno
Add widget tests exercising the navigateToRoute-based navigation introduced in this PR: brewery, drinks, favourites and drink-detail screens now have tests that trigger navigation to a drink detail and verify the destination renders. This raises patch coverage above the codecov/patch 70% threshold. https://claude.ai/code/session_01FqdVsDvtHXyWSszZA8tvno
richardthe3rd
force-pushed
the
claude/fix-drink-deep-links-uBA9g
branch
from
May 17, 2026 19:40
16102e2 to
47abd6e
Compare
Contributor
🚀 Cloudflare Pages PreviewYour preview deployment is ready! Preview URL: https://claude-fix-drink-deep-links.staging-cambeerfestival.pages.dev This preview will be automatically updated when you push new commits to this PR. |
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GoRouter's push() from within a ShellRoute does not update
currentConfiguration.uri on web, so the URL stayed stale after
tapping a drink card. Fix by introducing navigateToRoute(), which
calls context.go() on web (URL-updating) and context.push() on
mobile (stack-preserving).
Drink detail route now includes category segment
(/:festivalId/drink/:category/:id) for SEO and App Links alignment.
buildDrinkDetailPath gains a required category param; all call sites
updated (drinks_screen, drink_detail_screen, main, drink_list_section).
Share URLs now include the category segment so shared links resolve
correctly to the new route format.
New router_test covers that the URL contains all four segments after
navigation. E2e routing spec updated to the four-segment URL format.
https://claude.ai/code/session_01FqdVsDvtHXyWSszZA8tvno