Skip to content

[#536] Add immediate loading feedback across Blade navigation - #537

Open
DGoel1602 wants to merge 2 commits into
mainfrom
blade/responsive-navigation
Open

[#536] Add immediate loading feedback across Blade navigation#537
DGoel1602 wants to merge 2 commits into
mainfrom
blade/responsive-navigation

Conversation

@DGoel1602

@DGoel1602 DGoel1602 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Why

Blade can feel unresponsive between a click and the next server response. The previous link wrapper also added an 80 ms delay and left its exit state set after navigation. Moving between pages should acknowledge the destination immediately and remain usable while data loads.

What

Closes: #536

  • Use one persistent Blade navigation provider for links, programmatic navigation, and refresh feedback. Highlight the pending destination and show an accessible progress bar immediately; clear feedback when navigation completes, fails, or is interrupted.
  • Keep the application shell usable during transitions, reuse existing page skeletons, and add a Blade root skeleton for remaining route/layout waits.
  • Update already-loaded tabs and form section selection optimistically. Keep search/filter submissions in client navigation and close completed filter dialogs and mobile menus immediately.
  • Preserve unsaved-change guards, native link behavior, and reduced-motion support.

Scope is Blade plus its feature bundle and review media. Most component changes are imports routing existing links/router calls through the shared Blade provider. No other app or shared package source changed.

Test Plan

  • Blade Vitest suite: 799 tests passed across 142 files; the final search/filter adjustment also passed 220 targeted tests across 34 files.
  • Headed Playwright: 7/7 passed together, including held server responses, repeat/back navigation, 320px mobile/reduced motion, optimistic section selection, interrupted navigation, client search, unsaved settings, and filter-dialog completion.
  • Inspected desktop and 320px navigation/skeleton captures; checked for horizontal overflow.
  • pnpm format, pnpm lint, pnpm typecheck, and pnpm analyze:react:changed: passed. Lint reports existing repository warnings.
  • pnpm --filter=@forge/blade build: passed with temporary local-only values for the locally missing JUDGING_ACCESS_SECRET and NEXT_PUBLIC_BLADE_URL. No environment files or deployment settings changed.

To repeat the browser checks with a local test database, run pnpm --filter=@forge/blade run e2e responsive-navigation.spec.ts --headed. To check manually, delay route requests, navigate Forms → Members, use back and repeat; at 320px, select Members from the menu and confirm it closes while feedback remains visible. Also apply an issue filter and attempt navigation with unsaved profile changes.

Screenshots and recordings

The recordings use synthetic fixtures in an isolated local database. Tests deliberately hold route responses; these demonstrate feedback behavior, not production latency. Video capture adds 100 ms between browser actions for readability.

Desktop: pending destination and retained page Mobile: menu dismissed, static reduced-motion progress
Blade desktop navigation while Members loads Blade mobile navigation while Members loads

Desktop navigation

navigation-desktop.mp4

Mobile navigation

navigation-mobile.mp4
Root skeleton at desktop and 320px Blade desktop root loading skeleton Blade mobile root loading skeleton

The real root loading component was rendered through a temporary preview route for these captures; that route was removed. Capture notes.

Checklist

  • Database: No schema changes, OR I ran pnpm db:generate and committed the generated files in packages/db/drizzle/
  • Environment Variables: No environment variables changed, OR I have contacted the Development Lead to modify them on Coolify BEFORE merging.

Co-authored-by: Codex <codex@openai.com>
@DGoel1602 DGoel1602 added Feature New Feature or Request Major Big change - 2+ reviewers required Blade Change modifies code in Blade app labels Sep 6, 2026
@DGoel1602 DGoel1602 self-assigned this Sep 6, 2026
Co-authored-by: Codex <codex@openai.com>
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Blade now uses shared transition-aware links, routers, forms, loading surfaces, and navigation path state. Components provide optimistic tab and selector updates. Loading and reduced-motion feedback were added. Playwright and Vitest coverage verifies responsive navigation, interruptions, failures, forms, filters, cancellation, and route transitions.

Merge Risk: 🔵 Low · up to f55e1

The PR is broadly mergeable, but two navigation interactions can fail to show immediate pending feedback. Both fixes are localized and low risk.

🚥 Pre-merge checks | ✅ 5 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 50 files. (23 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
No Typescript Escape Hatches ⚠️ Warning The PR adds four TypeScript assertion escape hatches: release!:, requested!:, resolve!:, and reject!: in apps/blade/src/tests/e2e/responsive-navigation.spec.ts:80-81 and `apps/blade/src/test… Replace each !: declaration with an explicitly typed variable that allows undefined, then guard it before invocation and before returning it. This removes the definite-assignment assertions while preserving the deferred callback behavio…
Linked Issues check ❓ Inconclusive The code changes address the linked issue requirements for immediate feedback, optimistic navigation, loading states, guards, reduced motion, and Blade-only scope. The required screenshot and video fi… Provide reviewable evidence for the excluded files: navigation-desktop.mp4, navigation-desktop.png, navigation-mobile.mp4, navigation-mobile.png, skeleton-desktop.png, and skeleton-mobile.png. These files are excluded by the !**/*.mp4 and !…
✅ Passed checks (5 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The changes are limited to Blade implementation, Blade tests, feature documentation, and review evidence related to responsive navigation. No unrelated application, shared package, schema, dependency,…
No Hardcoded Secrets ✅ Passed No hardcoded API keys, passwords, tokens, or secrets were introduced. Scans of all 118 changed files and binary evidence found no known credential formats or secret assignments. The only secret-relate…
Validated Env Access ✅ Passed The feature commit a1badb2 adds no process.env lines. None of its 85 changed files contains process.env. The raw environment access found elsewhere is pre-existing, and the Blade process.env r…
Title check ✅ Passed The title starts with issue number [#536], clearly describes immediate Blade navigation loading feedback, and is 61 characters long.
Description check ✅ Passed The description directly explains the Blade navigation changes, scope, validation results, and test plan.
Full details: Linked Issues check

Explanation

The code changes address the linked issue requirements for immediate feedback, optimistic navigation, loading states, guards, reduced motion, and Blade-only scope. The required screenshot and video files are excluded from review, so their contents cannot be verified.

Resolution

Provide reviewable evidence for the excluded files: navigation-desktop.mp4, navigation-desktop.png, navigation-mobile.mp4, navigation-mobile.png, skeleton-desktop.png, and skeleton-mobile.png. These files are excluded by the !/*.mp4 and !/*.png path filters.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 50 files. (23 skipped: 6 unsupported, 17 over the file limit.)

Full details: No Typescript Escape Hatches

Explanation

The PR adds four TypeScript assertion escape hatches: release!:, requested!:, resolve!:, and reject!: in apps/blade/src/tests/e2e/responsive-navigation.spec.ts:80-81 and apps/blade/src/tests/loading/route-transitions.test.tsx:48-49. The parent revision contains none of these declarations. No added any types or @ts-ignore/@ts-expect-error directives were found.

Resolution

Replace each !: declaration with an explicitly typed variable that allows undefined, then guard it before invocation and before returning it. This removes the definite-assignment assertions while preserving the deferred callback behavior.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch blade/responsive-navigation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (2)
apps/blade/src/app/_components/shared/route-transition-link.tsx-156-156 (1)

156-156: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Replace group-data-[exiting=true] with group-data-[pending=true] in member-profile-settings-form.tsx. RouteTransitionLink now emits data-pending, so the existing selector never matches.

apps/blade/src/app/_components/admin/hackathon-events/hackathon-check-in-workspace.tsx-42-42 (1)

42-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use optimistic IDs for both selects. useNavigationRouter().replace defers the URL update in a transition, while both select values remain derived from searchParams. Their visible values can revert to the previous IDs until navigation commits. Add local optimistic IDs, update them with replaceSelection, and bind both value props to those IDs, as AdminFormsSectionSelect does.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: QUIET

Plan: Team

Run ID: c2c023dc-c7a2-4a1d-8499-9c7da547863f

📥 Commits

Reviewing files that changed from the base of the PR and between bd97fcc and f55e1d7.

⛔ Files ignored due to path filters (6)
  • .forge/features/blade-responsive-navigation/evidence/navigation-desktop.mp4 is excluded by !**/*.mp4
  • .forge/features/blade-responsive-navigation/evidence/navigation-desktop.png is excluded by !**/*.png
  • .forge/features/blade-responsive-navigation/evidence/navigation-mobile.mp4 is excluded by !**/*.mp4
  • .forge/features/blade-responsive-navigation/evidence/navigation-mobile.png is excluded by !**/*.png
  • .forge/features/blade-responsive-navigation/evidence/skeleton-desktop.png is excluded by !**/*.png
  • .forge/features/blade-responsive-navigation/evidence/skeleton-mobile.png is excluded by !**/*.png
📒 Files selected for processing (79)
  • .forge/features/blade-responsive-navigation/evidence/README.md
  • .forge/features/blade-responsive-navigation/spec.md
  • .forge/features/blade-responsive-navigation/srd.md
  • .forge/features/blade-responsive-navigation/status.md
  • .forge/features/blade-responsive-navigation/test-cases.md
  • apps/blade/src/app/_components/admin/alumni/alumni-bulletin-admin.tsx
  • apps/blade/src/app/_components/admin/analytics/analytics-dashboard.tsx
  • apps/blade/src/app/_components/admin/analytics/hackathon-analytics-dashboard.tsx
  • apps/blade/src/app/_components/admin/analytics/team-performance-analytics-dashboard.tsx
  • apps/blade/src/app/_components/admin/companies/company-admin-dashboard.tsx
  • apps/blade/src/app/_components/admin/companies/company-admin-detail.tsx
  • apps/blade/src/app/_components/admin/email/email-portal-admin.tsx
  • apps/blade/src/app/_components/admin/email/email-portal-workspace.tsx
  • apps/blade/src/app/_components/admin/events/event-admin-dashboard.tsx
  • apps/blade/src/app/_components/admin/events/event-feedback-template-editor.tsx
  • apps/blade/src/app/_components/admin/events/event-workspace-sections.tsx
  • apps/blade/src/app/_components/admin/forms/admin-form-builder.tsx
  • apps/blade/src/app/_components/admin/forms/admin-forms-dashboard.tsx
  • apps/blade/src/app/_components/admin/forms/admin-forms-section-select.tsx
  • apps/blade/src/app/_components/admin/forms/form-responses-dashboard.tsx
  • apps/blade/src/app/_components/admin/forms/form-sections-manager.tsx
  • apps/blade/src/app/_components/admin/hackathon-events/hackathon-check-in-workspace.tsx
  • apps/blade/src/app/_components/admin/hackathon-events/hackathon-events-workspace.tsx
  • apps/blade/src/app/_components/admin/hackathon/hackathon-detail.tsx
  • apps/blade/src/app/_components/admin/hackathon/hackathon-list.tsx
  • apps/blade/src/app/_components/admin/hackathon/hackers/hacker-roster.tsx
  • apps/blade/src/app/_components/admin/hackathon/hackers/use-roster-url-state.ts
  • apps/blade/src/app/_components/admin/issues/issue-create-dialog.tsx
  • apps/blade/src/app/_components/admin/issues/issue-detail.tsx
  • apps/blade/src/app/_components/admin/issues/issue-filters.tsx
  • apps/blade/src/app/_components/admin/issues/issue-views.tsx
  • apps/blade/src/app/_components/admin/issues/issue-workspace-dock.tsx
  • apps/blade/src/app/_components/admin/issues/issue-workspace.tsx
  • apps/blade/src/app/_components/admin/issues/template-catalog-dialog.tsx
  • apps/blade/src/app/_components/admin/logs/admin-logs-dashboard.tsx
  • apps/blade/src/app/_components/admin/members/member-admin-dashboard.tsx
  • apps/blade/src/app/_components/admin/roles/admin-config-console.tsx
  • apps/blade/src/app/_components/admin/roles/role-detail-dialog.tsx
  • apps/blade/src/app/_components/admin/roles/role-management-dashboard.tsx
  • apps/blade/src/app/_components/auth/sign-out-button.tsx
  • apps/blade/src/app/_components/forms/generic-form-respondent.tsx
  • apps/blade/src/app/_components/judging/evaluation-dialog.tsx
  • apps/blade/src/app/_components/judging/guest-name-gate.tsx
  • apps/blade/src/app/_components/judging/judge-deliberation.tsx
  • apps/blade/src/app/_components/judging/judging-configuration-panel.tsx
  • apps/blade/src/app/_components/judging/judging-control-panel.tsx
  • apps/blade/src/app/_components/judging/project-command-center.tsx
  • apps/blade/src/app/_components/member/alumni-dashboard.tsx
  • apps/blade/src/app/_components/member/dashboard-client.tsx
  • apps/blade/src/app/_components/member/member-dashboard.tsx
  • apps/blade/src/app/_components/member/member-dues-payment.tsx
  • apps/blade/src/app/_components/member/member-event-feedback.tsx
  • apps/blade/src/app/_components/member/member-events-dashboard.tsx
  • apps/blade/src/app/_components/member/member-form-history.tsx
  • apps/blade/src/app/_components/member/member-profile-settings-form.tsx
  • apps/blade/src/app/_components/member/member-signup-form.tsx
  • apps/blade/src/app/_components/projects/admin-project-workspace.tsx
  • apps/blade/src/app/_components/projects/judge-project-workspace.tsx
  • apps/blade/src/app/_components/projects/project-directory.tsx
  • apps/blade/src/app/_components/providers.tsx
  • apps/blade/src/app/_components/public/member-landing-support.tsx
  • apps/blade/src/app/_components/shared/account-settings-link.tsx
  • apps/blade/src/app/_components/shared/authenticated-shell.tsx
  • apps/blade/src/app/_components/shared/desktop-admin-navigation.tsx
  • apps/blade/src/app/_components/shared/mobile-admin-navigation.tsx
  • apps/blade/src/app/_components/shared/route-transition-link.tsx
  • apps/blade/src/app/globals.css
  • apps/blade/src/app/judge/access-error/page.tsx
  • apps/blade/src/app/loading.tsx
  • apps/blade/src/app/not-found.tsx
  • apps/blade/src/app/sponsor/page.tsx
  • apps/blade/src/tests/admin/alumni-admin-navigation.test.tsx
  • apps/blade/src/tests/admin/authenticated-shell.test.tsx
  • apps/blade/src/tests/admin/company-admin-navigation.test.tsx
  • apps/blade/src/tests/admin/email-portal-navigation.test.tsx
  • apps/blade/src/tests/admin/hackathon-admin-navigation.test.tsx
  • apps/blade/src/tests/e2e/responsive-navigation.spec.ts
  • apps/blade/src/tests/events/admin-event-navigation.test.tsx
  • apps/blade/src/tests/loading/route-transitions.test.tsx
💤 Files with no reviewable changes (6)
  • apps/blade/src/tests/admin/hackathon-admin-navigation.test.tsx
  • apps/blade/src/tests/admin/email-portal-navigation.test.tsx
  • apps/blade/src/tests/admin/company-admin-navigation.test.tsx
  • apps/blade/src/tests/events/admin-event-navigation.test.tsx
  • apps/blade/src/tests/admin/authenticated-shell.test.tsx
  • apps/blade/src/tests/admin/alumni-admin-navigation.test.tsx

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Blade Change modifies code in Blade app Feature New Feature or Request Major Big change - 2+ reviewers required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Blade navigation respond immediately

1 participant