Skip to content

fix(shell): prevent duplicate route headers#414

Merged
ucguy4u merged 2 commits into
mainfrom
codex/issue-398-header-ownership
Jun 28, 2026
Merged

fix(shell): prevent duplicate route headers#414
ucguy4u merged 2 commits into
mainfrom
codex/issue-398-header-ownership

Conversation

@ucguy4u

@ucguy4u ucguy4u commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR introduces changes to shell and route header ownership.
Goal: ensure shell-mounted routes show only one visible top header.

Core outcome:

  • shell-managed routes keep the shared AppShell chrome
  • custom-header and nested flows hide the shell app bar instead of rendering duplicates
  • provider and router tests lock the ownership behavior

Changes

Code

  • Added:
    • app/test/core/app/app_shell_header_ownership_test.dart router-level shell/header coverage
  • Updated:
    • app/lib/core/providers/navigation_provider.dart with route-to-header ownership rules
    • app/lib/core/app/app_shell.dart to gate shell chrome by current route
    • app/lib/core/routing/app_router.dart to pass the active path into the shell
    • app/test/core/providers/navigation_provider_test.dart with shell-vs-route ownership assertions

Logic

  • /money, /mind, /mind/chat, /mind/models, and /games remain shell-owned
  • custom-header routes like /music, /iptv, /quest, /mind/profile, and nested money flows become route-owned
  • fullscreen behavior remains unchanged and still bypasses shell chrome entirely

API Changes (if any)

  • None.

Database Changes (if any)

  • None.

Observability / Logging

  • None.

Performance Impact

  • Latency: no meaningful change
  • Throughput: no meaningful change
  • Memory/CPU: negligible

Risks

  • route ownership depends on path classification, so new shell routes must join the config map
  • any future route without a local app bar must be marked shell-owned explicitly
  • Rollback plan:
    • revert this PR

Testing

  • Unit tests:
    • navigation provider route ownership coverage
  • Integration tests:
    • minimal router widget test for shell-owned root, route-owned root, and nested route-owned flow
  • Manual testing:
    • not run

Deployment Notes

  • Config changes:
    • none
  • Order of deployment:
    • standard

Related Commits

  • fix(shell): prevent duplicate route headers

Notes

  • Review the exact ownership map in navigation_provider.dart if additional shell-mounted routes are expected.

Closes #398

- add route-to-header ownership rules for shell-mounted paths
- hide shell chrome when a route owns its own app bar
- add provider and router tests for shell and nested header behavior

Removes duplicate headers on shell routes without stripping route-specific actions.
@github-actions

Copy link
Copy Markdown

Plugin Module Size Gate

Policy: modules over 3 MB must be delivered as plugins; plugin modules over 5 MB must document cache management.

Module Size Type Status
No changed Dart/Flutter modules detected n/a n/a OK

- update canonical tab routes in the navigation wiki
- explain which shell-mounted routes use shell chrome vs local app bars
- document the legacy redirects for mind, beats, and stream

Keeps user-facing navigation docs aligned with the header ownership fix.
@ucguy4u ucguy4u merged commit 23331e0 into main Jun 28, 2026
1 check passed
@ucguy4u ucguy4u deleted the codex/issue-398-header-ownership branch June 28, 2026 05:18
@github-actions

Copy link
Copy Markdown

🚀 PR Quick Check Summary

Check Status Description
PR Validation ❌ failure Title format, docs, bundled model guardrail
Code Quality ❌ cancelled Analyze, formatting
Core Tests ✅ success Core package unit tests

💡 Note: Full app tests, coverage reports, and security scans run on merge to main.

View Details

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

🚀 PR Quick Check Summary

Check Status Description
PR Validation ✅ success Title format, docs, bundled model guardrail
Code Quality ❌ failure Analyze, formatting
Core Tests ✅ success Core package unit tests

💡 Note: Full app tests, coverage reports, and security scans run on merge to main.

View Details

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.

[AGENT] UIUX-002: Consolidate shell and page header ownership

1 participant