Review festival linking progress and plan next steps - #194
Merged
Conversation
Implement Option A: Festival context displayed in breadcrumbs and screen headers Changes: - Breadcrumbs now show festival ID (e.g., "cbf2025") instead of "Drinks" - BreweryScreen: Added "at [Festival Name]" subtitle under brewery name - StyleScreen: Added "[Style] at [Festival Name]" heading in header - DrinkDetailScreen: Updated breadcrumb to show festival ID - Updated golden images for screenshot tests - Fixed style_screen_test.dart assertions for new breadcrumb format All 459 tests passing.
Complete consistency across all detail screens by adding "at [Festival Name]" to the drink detail header. Changes: - DrinkDetailScreen: Added "at Cambridge Beer Festival 2025" subtitle below drink name in header (matches BreweryScreen pattern) - Updated golden images for screenshot tests - All 476 tests passing Now all three detail screens show festival context consistently: - DrinkDetailScreen: "Drink Name" → "at Festival" → Brewery - BreweryScreen: "Brewery Name" → "at Festival" - StyleScreen: "Style at Festival" heading
- Convert style names to lowercase in buildStylePath() for SEO optimization - Update StyleScreen to use case-insensitive filtering for style matching - Update navigation_helpers tests to expect lowercase URLs - Accept any case for navigation but lowercase is canonical format All 477 tests passing.
- Updated docs/code/routing.md: - Listed all routes with festival scoping - Added Style URL Canonicalization section explaining lowercase format - Updated Deep Link Navigation to explain breadcrumb pattern - Updated docs/ui-components.md: - Updated BreadcrumbBar usage examples to show festival ID pattern - Added current pattern explanation for festival-scoped routing - Updated docs/planning/deep-linking/phase-0-guide.md: - Updated buildStylePath() function docs with lowercase logic - Updated all test examples to expect lowercase URLs - Updated example URL paths to show lowercase style URLs - Updated docs/navigation.md: - Added comment showing buildStylePath() returns lowercase URLs All examples now reflect current implementation with: - Festival-scoped routing (/:festivalId/...) - Breadcrumbs showing festival ID - Headers showing "at Festival Name" - Lowercase canonical style URLs
Updated tests to expect lowercase style URLs matching the canonical format: - test/router_test.dart: Updated style path test to expect lowercase 'ipa' - test-e2e/routing.spec.ts: Updated E2E test to use lowercase 'american ipa' Documentation fixes for consistency: - docs/navigation.md: Updated style URL example to lowercase - CLAUDE.md: Updated screenshots config example to lowercase - docs/planning/deep-linking/PHASE-1-COMPLETE.md: Updated manual testing checklist - docs/planning/deep-linking/design.md: Updated example URLs and design decisions - docs/planning/deep-linking/implementation-plan.md: Updated JSON config example - docs/planning/deep-linking/testing-strategy.md: Updated test examples - docs/planning/deep-linking/design-review.md: Updated edge case examples All 477 tests passing. Ready for CI.
Contributor
LCOV of commit
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
🚀 Cloudflare Pages PreviewYour preview deployment is ready! Preview URL: https://claude-review-festival-progr.staging-cambeerfestival.pages.dev This preview will be automatically updated when you push new commits to this PR. |
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
2 participants
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.
This pull request implements canonical lowercase URLs for style detail pages and updates navigation and UI patterns to support festival-scoped routing. The changes ensure consistent, SEO-friendly, and shareable URLs, update documentation and tests accordingly, and improve the user interface to reflect the current festival context on detail screens.
URL Canonicalization and Routing:
/cbf2025/style/ipainstead of/cbf2025/style/IPA), ensuring consistency and SEO benefits across the app. All navigation helpers, documentation, and tests have been updated to reflect this change. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]UI/UX Improvements for Festival Context:
Case-Insensitive Style Matching:
Documentation and Testing Updates:
Test and Config File Updates:
screenshots.config.json) and Playwright/E2E test scripts have been updated to use lowercase style URLs, ensuring alignment with the new routing scheme. [1] [2] [3]These changes together provide a more robust, user-friendly, and future-proof routing and navigation experience for festival detail pages.