UX Rescue Phase 0: product-wide audit, target IA, and rescue roadmap - #148
Merged
Conversation
Phase 0 of the UX Rescue initiative — a full audit of every actual route, component, and interaction pattern in the frontend (grounded in App.tsx's 62 declared routes, backend contracts affecting UX, and 5 parallel deep-dive investigations), producing: - Full screen/route inventory with a Keep/Polish/Restructure/Merge/Replace/ Advanced-fallback/Remove decision for every route, including confirmed dead routes, an orphaned page, and a real functional bug (campaign edit ignores its own route param). - Component/interaction pattern inventory identifying the near-total absence of a shared component library and the underused WorkspaceShell primitive. - 19 role-based personas grounded in actual permission codes and scopes. - Current journey maps for Notes, Form Designer, Form Operations, Facility Workspace, and other domains with measured (not guessed) screen/navigation counts. - Facility/Region/Headquarters scope and context model, documenting that the backend workspace framework already supports Region/Headquarters levels. - 30 root-caused friction points. - Gap analyses for Observation Workspace, Form Designer, and Form Operations, each ending in a concrete target flow feeding issues #143-#145. - Remaining-screens disposition matrix feeding #146. - Target information architecture, binding interaction patterns, a measurable metrics baseline, and a route migration/transition plan. - An executable rescue roadmap, re-ordering Region/Headquarters Workspace earlier than originally proposed since their architectural foundation already exists and those two personas are currently unserved by any working UI. - scripts/ux-route-inventory-check.mjs: a lightweight, non-fragile check (plain regex over App.tsx's static route declarations, not a dynamic router parser) verifying every route has a documented decision and every sidebar nav link resolves to a real route. No functional, domain, database, or API contract changes in this phase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @henter36, your pull request is larger than the review limit of 150000 diff characters
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughAdds a comprehensive Arabic UX Rescue Phase 0 documentation set covering current journeys, UI patterns, personas, friction, target architecture, domain gaps, route migration, and phased execution. It also adds automated route and context-panel inventory validation. ChangesUX Rescue audit
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Reviewer's GuideDocuments and codifies a comprehensive UX audit ("UX Rescue" Phase 0) of the frontend, including personas, route/screen inventory, gap analyses, target IA and roadmap, and adds a small automation script and npm task to keep the documented route inventory in sync with App.tsx and FacilityWorkspacePage.tsx. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (2)
docs/ux-rescue/migration-and-route-transition-plan.md (1)
64-64: 🔒 Security & Privacy | 🔵 TrivialUse dedicated route telemetry instead of
AuditLogby default.Page-route usage is not necessarily an auditable business action. Prefer access metrics or dedicated telemetry with explicit retention, sampling, and user-identifier handling; use
AuditLogonly if its schema and compliance policy explicitly support this workload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ux-rescue/migration-and-route-transition-plan.md` at line 64, Update the migration requirement to prioritize dedicated route-access telemetry or metrics, including explicit retention, sampling, and user-identifier handling. Treat AuditLog as a fallback only when its schema and compliance policy explicitly support route-usage tracking.docs/ux-rescue/observation-workspace-gap-analysis.md (1)
57-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the baseline metrics reproducible.
Add the measured persona/journey, counting rules (for example, whether panels count as screens and redirects count as route changes), and the source or sample behind
4-5,6-8, and≤ 2. Without that metadata, the targets cannot be validated consistently intask-metrics-baseline.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ux-rescue/observation-workspace-gap-analysis.md` around lines 57 - 63, Expand the measurement section around the metrics table with the measured persona and journey, explicit counting rules for screens, panels, and route changes or redirects, and the source or sample supporting the current values 4-5 and 6-8 and the target ≤ 2. Keep the existing metrics and targets unchanged while making their measurement reproducible for task-metrics-baseline.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ux-rescue/component-and-interaction-inventory.md`:
- Line 5: Update the shared-library description to state that listPageUtils.ts
contains four functions, matching the four listed symbols formatListDate,
listSortIndicator, nextListSortState, and listQueryErrorMessage. Apply this
correction in docs/ux-rescue/component-and-interaction-inventory.md line 5 and
docs/ux-rescue/friction-register.md line 18; no other content changes are
needed.
In `@docs/ux-rescue/current-user-journeys.md`:
- Line 12: Update the “إنشاء ونشر حملة نموذج” row in current-user-journeys.md so
its action count and detailed flow consistently include both creation and
publishing, or rename the row and scope description to creation only; keep the
journey metrics and stated scope aligned.
In `@docs/ux-rescue/form-designer-gap-analysis.md`:
- Around line 8-10: Reconcile the route and screen-count summaries across the
three specified documents, explicitly defining whether each total counts route
visits, unique routes, screens, or route families. In
docs/ux-rescue/form-designer-gap-analysis.md lines 8-10, change the summary to
count all seven listed stages as distinct routes; in
docs/ux-rescue/target-information-architecture.md lines 75-78, align “around 12”
with the listed note/corrective-action and nine form-operation routes; and in
docs/ux-rescue/remaining-screens-audit.md lines 73-82, include
NotificationsPage, FormCompliancePage, the four escalation settings pages, and
EscalationOccurrencesPage while labeling the counting unit.
In `@docs/ux-rescue/migration-and-route-transition-plan.md`:
- Line 28: Update the redirect guidance in the migration plan to describe
<Navigate> as an in-app client-side redirect rather than an HTTP 301/302
response. Separately document that server/CDN configuration is required when
actual HTTP redirect status semantics are needed, while preserving the
requirement that legacy links remain usable.
- Around line 29-30: Update the route resolution and redirect targets for merged
routes such as `/notes/:id` and `/corrective-actions/:id` so rollback resolves
deep links to the legacy page rather than merely hiding Workspace navigation.
Keep the legacy route components registered and available until rollback support
is removed, while preserving the Workspace Panel/Split View target when rollback
is inactive.
In `@docs/ux-rescue/personas-and-jobs-to-be-done.md`:
- Line 267: Correct the malformed WorkspaceShell reference in the documentation
by replacing the stray E-prefixed backtick text with the properly formatted
`WorkspaceShell` reference, leaving the surrounding content unchanged.
In `@docs/ux-rescue/remaining-screens-audit.md`:
- Line 56: Remove the proposed Region-row link to `/facilities?regionId=` from
the RegionsPage audit entry until region filtering is implemented; retain the
note that the current facilities flow in FacilitiesPage calls
`api.facilities(undefined, search)` and cannot preserve a selected region.
In `@docs/ux-rescue/rescue-roadmap.md`:
- Line 5: Update docs/ux-rescue/rescue-roadmap.md lines 5-5 to describe existing
Region/Headquarters infrastructure as reusable foundations, not as evidence that
implementation is limited to a WorkspaceDefinitionProvider and page. Update
lines 104-104 to include Region aggregation, widget-provider implementation,
dependency-injection registration, drill-down behavior, and associated tests;
apply the same scope correction to Headquarters on lines 118-118.
- Around line 81-86: حدّث قسم «حدود الـPR» ومعايير القبول وخطة
المخاطر/الاختبارات في خارطة الطريق لتوضيح أن اعتماد MasterDetailWorkspaceLayout
قد يغيّر سلوك التنقل على الهاتف، بما في ذلك إجراء الرجوع، أو احذف هذا الاعتماد
للحفاظ على نطاق إعادة الهيكلة فقط. اجعل معايير القبول وخطة الانحدار تغطي سلوك
التنقل الجديد صراحةً إذا أبقيت التغيير.
In `@docs/ux-rescue/screen-and-route-inventory.md`:
- Line 124: Update the documented checker command in the route inventory
documentation to invoke the JavaScript script with Node instead of Bash, using
node src/frontend/scripts/ux-route-inventory-check.mjs or the existing npm run
check:ux-routes command.
In `@docs/ux-rescue/target-information-architecture.md`:
- Around line 53-66: The target architecture document must define compatibility
behavior for merged routes: specify legacy-to-canonical redirects and
deprecation handling for /notes and the merged form/campaign routes, including
the destination for /form-campaigns/:campaignId/edit. Clarify whether
/form-compliance/* remains independently routable or is only embedded in Form
Operations Workspace, and document these outcomes alongside the route mappings.
- Around line 15-25: عدّل عنوان قسم "الفرق بين الأنواع الخمسة" ليتوافق مع فئات
الجدول السبع الحالية، أو أعد هيكلة الجدول إلى خمس فئات واضحة؛ احرص على أن يعكس
العنوان والتصنيف النهائيان نفس عدد الأنواع وأن تبقى جميع الفئات المذكورة قابلة
للاستخدام في قرارات التنفيذ.
In `@docs/ux-rescue/task-metrics-baseline.md`:
- Line 13: Correct the “عدد مرات فقدان الفلاتر عبر الرحلة” metric so it matches
the evidence that filters are restored correctly: set the confirmed count to
zero. Keep the existing list-position loss observation separate rather than
counting it as filter loss.
- Line 31: Update the “عدد مرات فقد البيانات المحتملة (نافذة تعرّض)” metric to
avoid presenting 800ms as the complete exposure window: either rename it to
explicitly describe autosave/retry latency, or redefine it to include pending or
failed writes and navigation while dirty or in error/conflict states. Ensure the
baseline and target values use the same clarified metric.
In `@docs/ux-rescue/workspace-scope-and-context-model.md`:
- Around line 60-65: Update the documented creation-form contract so the server
derives or validates facilityId from the authorized WorkspaceContext or route
context, never trusting a client-controlled hidden field as the scope source.
Treat any client field only as presentation state, and preserve the same
requirement for future embedded creation forms.
In `@src/frontend/scripts/ux-route-inventory-check.mjs`:
- Around line 101-130: Update the ux-route inventory check to derive known panel
types by parsing the authoritative PANEL_TYPES declaration in
FacilityWorkspacePage.tsx instead of maintaining the divergent KNOWN_PANEL_TYPES
set. Reconcile the extracted declaration with documented panel types, while
preserving the existing use-site scanning and ensuring newly declared types and
inventory entries are validated.
- Around line 74-77: Update the dead-route validation around deadRoutesSection
to parse the section’s route rows instead of only checking for the heading.
Require at least one listed dead route, then verify every entry matches a
declared route and has a “Remove/redirect” disposition, failing validation for
missing, undeclared, or incorrectly classified entries.
---
Nitpick comments:
In `@docs/ux-rescue/migration-and-route-transition-plan.md`:
- Line 64: Update the migration requirement to prioritize dedicated route-access
telemetry or metrics, including explicit retention, sampling, and
user-identifier handling. Treat AuditLog as a fallback only when its schema and
compliance policy explicitly support route-usage tracking.
In `@docs/ux-rescue/observation-workspace-gap-analysis.md`:
- Around line 57-63: Expand the measurement section around the metrics table
with the measured persona and journey, explicit counting rules for screens,
panels, and route changes or redirects, and the source or sample supporting the
current values 4-5 and 6-8 and the target ≤ 2. Keep the existing metrics and
targets unchanged while making their measurement reproducible for
task-metrics-baseline.md.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e41f1040-ee74-4a49-9284-b398ee23f105
📒 Files selected for processing (17)
docs/ux-rescue/component-and-interaction-inventory.mddocs/ux-rescue/current-user-journeys.mddocs/ux-rescue/form-designer-gap-analysis.mddocs/ux-rescue/form-operations-gap-analysis.mddocs/ux-rescue/friction-register.mddocs/ux-rescue/interaction-patterns.mddocs/ux-rescue/migration-and-route-transition-plan.mddocs/ux-rescue/observation-workspace-gap-analysis.mddocs/ux-rescue/personas-and-jobs-to-be-done.mddocs/ux-rescue/remaining-screens-audit.mddocs/ux-rescue/rescue-roadmap.mddocs/ux-rescue/screen-and-route-inventory.mddocs/ux-rescue/target-information-architecture.mddocs/ux-rescue/task-metrics-baseline.mddocs/ux-rescue/workspace-scope-and-context-model.mdsrc/frontend/package.jsonsrc/frontend/scripts/ux-route-inventory-check.mjs
Fixes 17 findings from the first review pass on PR #148, all verified against actual code before changing anything: - ux-route-inventory-check.mjs: derive known Context Panel types from FacilityWorkspacePage.tsx's own PANEL_TYPES declaration (33 types) instead of a hand-maintained allowlist that had already drifted (missing 13 real types, including 8 sensitive-custody sub-entities); make the dead-routes check actually parse and validate route rows instead of only checking that the section heading exists. - screen-and-route-inventory.md: correct the Context Panel table to reflect the full 33-type PANEL_TYPES union grouped by domain (previously only 15 types found via narrower call-site patterns); fix "bash ...mjs" to "node ...mjs" in the verification command; add a missing route row (/form-campaigns/:campaignId) the checker caught during this same pass. - Arithmetic/wording corrections: listPageUtils.ts has 4 functions not 3 (component-and-interaction-inventory.md, friction-register.md); form designer's 7 journey stages are 7 distinct routes not 6 (form-designer-gap-analysis.md, current-user-journeys.md, task-metrics-baseline.md); campaign journey row now counts both create and publish; reconciled route-merge totals and disambiguated "screen" vs "route" as counting units (target-information-architecture.md, remaining-screens-audit.md); split "filter loss" from "list-position loss" since the parenthetical already said filters restore correctly (task-metrics-baseline.md); reframed the 800ms autosave figure as retry latency, not the only data-loss exposure window. - Correctness fixes: <Navigate> is a client-side redirect, not an HTTP 301/302 response; rollback must operate at the route resolver level, not just by hiding a nav link, since deep links/notifications would otherwise keep resolving to the new experience (migration-and-route-transition-plan.md, target-information-architecture.md); qualified Region/Headquarters backend scope to include widget providers, DI registration, and aggregation logic, not just a WorkspaceDefinitionProvider (rescue-roadmap.md); separated the Facility Workspace closeout phase's header-only refactor from the behavior-changing MasterDetailWorkspaceLayout adoption (mobile back button) it had been bundled with under a "refactor-only" claim; removed an unworkable /facilities?regionId= link suggestion since FacilitiesPage ignores region filters today; corrected a hidden-field description to state plainly that client-supplied values are never authoritative for scope, regardless of their initial source (workspace-scope-and-context-model.md); fixed a malformed `WorkspaceShell` reference typo (personas-and-jobs-to-be-done.md). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
10 tasks
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
1 participant
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.
ملخص
مرحلة صفر من UX Rescue: دراسة تنفيذية معمارية كاملة لتجربة المستخدم عبر التطبيق بأكمله — لا تنفيذ جزئي للتصميم الجديد، ولا تغييرات وظيفية أو في Domain/قاعدة البيانات/عقود API.
App.tsxمع قرار واضح لكل واحد (Keep/Polish/Restructure/Merge/Replace/Advanced fallback/Remove)، بما في ذلك اكتشاف Routes ميتة، مكوّن يتيم (NotesListPage.tsx)، وخلل وظيفي حقيقي (تعديل حملة نشر يتجاهل معرّفها تمامًا).scripts/ux-route-inventory-check.mjs) تتحقق من تطابق الجرد مع الكود الفعلي.Test plan
dotnet build src/backend/Baseera.slnx -c Release— نجحdotnet test .../Baseera.UnitTests— 932/932 نجحnpm run typecheck/npm run lint— نظيفnpm run test— 283/283 نجحnpm run build— نجحnpm audit --audit-level=high— 0 ثغراتbash scripts/check-nuget-vulnerabilities.sh— 0 ثغراتgitleaks detect— لا تسريباتnode src/frontend/scripts/ux-route-inventory-check.mjs— نجح (62 route متطابق تمامًا)git diff --check— نظيفCloses #142
Prepares #143, #144, #145 and #146
Related to #11, #12, #13 and #21
🤖 Generated with Claude Code
Summary by Sourcery
Document and formalize a product-wide UX audit and rescue roadmap, including route inventory tooling and personas, without changing runtime behavior.
Enhancements:
Documentation:
Summary by CodeRabbit