💄(frontend) redesign error pages with Figma layout#2528
Conversation
Add SVGs for 404, 500, 503, access denied, and email confirmation pages.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| <ErrorActionLink | ||
| onClick={() => | ||
| safeTarget | ||
| ? window.location.assign(safeTarget) |
|
Size Change: +11.3 kB (+0.26%) Total Size: 4.38 MB 📦 View Changed
|
Ovgodd
changed the title
857da66 to
4421482
Compare
Use SVG icons, 102x72 layout, 12px description and text-link actions.
Wire new SVG icons and show Home plus Refresh actions on error pages.
Apply new illustration, typography and text-link actions for access errors.
Align 503 offline page and user reconciliation screen with Figma design.
Apply consistent flex centering and replace Material home icon in ErrorPage.
Use Cunningham button on 401 and ui-kit icons with brand tertiary on 403.
Align e2e assertions with redesigned access denied pages.
Export action and description styles from ErrorPage and simplify offline page.
Use Docs logo header and bottom actions without sidebar on error pages.
1b44ae9 to
22638bb
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR redesigns frontend error pages around shared Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@src/frontend/apps/impress/src/features/auth/components/UserReconciliation.tsx`:
- Line 65: Update the useUserReconciliationsQuery call to destructure its
refetch function, then replace the ErrorActionLink window.location.reload
callback with refetch so resending the email re-triggers the managed query
without a full page reload.
In
`@src/frontend/apps/impress/src/features/docs/doc-management/components/DocPage403.tsx`:
- Around line 69-71: Update the title element in DocPage403 so its translated
“Access denied” and “Docs” values are combined into one interpolated string
rather than separate JSX children, preserving the existing title text and
separator.
- Around line 130-133: Update the useCreateDocAccessRequest destructuring to
capture its mutation-in-progress state, then combine that state with
hasRequested in RequestAccessButton’s disabled condition so rapid clicks cannot
dispatch duplicate requests.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3fa7b2ba-4097-4894-b3be-b3a177af0064
⛔ Files ignored due to path filters (8)
src/frontend/apps/impress/src/assets/icons/404.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/500.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/503.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/DOCS.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/Docs Locked.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/Mail.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/bubble-text.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/retry.svgis excluded by!**/*.svg
📒 Files selected for processing (18)
CHANGELOG.mdsrc/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/utils-share.tssrc/frontend/apps/impress/src/components/ErrorPage.tsxsrc/frontend/apps/impress/src/components/ErrorPageFooter.tsxsrc/frontend/apps/impress/src/components/ErrorPageHeader.tsxsrc/frontend/apps/impress/src/components/index.tssrc/frontend/apps/impress/src/features/auth/components/UserReconciliation.tsxsrc/frontend/apps/impress/src/features/docs/doc-management/components/DocPage403.tsxsrc/frontend/apps/impress/src/layouts/PageLayout.tsxsrc/frontend/apps/impress/src/pages/401.tsxsrc/frontend/apps/impress/src/pages/404.tsxsrc/frontend/apps/impress/src/pages/500.tsxsrc/frontend/apps/impress/src/pages/_error.tsxsrc/frontend/apps/impress/src/pages/offline/index.tsxsrc/frontend/apps/impress/src/pages/user-reconciliations/active/[id]/index.tsxsrc/frontend/apps/impress/src/pages/user-reconciliations/inactive/[id]/index.tsx
| $gap="0.5rem" | ||
| $css="margin-top: 20px;" | ||
| > | ||
| <ErrorActionLink onClick={() => window.location.reload()}> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Avoid a full page reload for resending the email.
Calling window.location.reload() forces a hard browser refresh, which disrupts the SPA experience. Since useUserReconciliationsQuery is managing the data fetch (and likely triggering the email), you can extract its refetch function to gracefully re-trigger the API request.
♻️ Proposed fix
First, extract refetch from your query on line 26:
const { data: userReconciliations, isError, refetch } = useUserReconciliationsQuery({
type,
reconciliationId,
});Then, replace the hard reload with refetch:
- <ErrorActionLink onClick={() => window.location.reload()}>
+ <ErrorActionLink onClick={() => void refetch()}>🤖 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
`@src/frontend/apps/impress/src/features/auth/components/UserReconciliation.tsx`
at line 65, Update the useUserReconciliationsQuery call to destructure its
refetch function, then replace the ErrorActionLink window.location.reload
callback with refetch so resending the email re-triggers the managed query
without a full page reload.
| <title> | ||
| {t('Access Denied - Error 403')} - {t('Docs')} | ||
| {t('Access denied')} - {t('Docs')} | ||
| </title> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Format the <title> content as a single string.
next/head can issue warnings or format text incorrectly when <title> contains multiple children spread across lines. It's safer to use template literals to guarantee a single text string.
♻️ Proposed fix
- <title>
- {t('Access denied')} - {t('Docs')}
- </title>
+ <title>{`${t('Access denied')} - ${t('Docs')}`}</title>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <title> | |
| {t('Access Denied - Error 403')} - {t('Docs')} | |
| {t('Access denied')} - {t('Docs')} | |
| </title> | |
| <title>{`${t('Access denied')} - ${t('Docs')}`}</title> |
🤖 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
`@src/frontend/apps/impress/src/features/docs/doc-management/components/DocPage403.tsx`
around lines 69 - 71, Update the title element in DocPage403 so its translated
“Access denied” and “Docs” values are combined into one interpolated string
rather than separate JSX children, preserving the existing title text and
separator.
| <RequestAccessButton | ||
| onClick={() => createRequest({ docId: id, role: Role.EDITOR })} | ||
| disabled={hasRequested} | ||
| > |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Disable the button while the request mutation is in progress.
Currently, the button is only disabled once hasRequested becomes true (which happens after the query refetch completes). If a user clicks rapidly, multiple API requests could be dispatched. Destructuring the mutation's loading state and adding it to the disabled condition will prevent this.
♻️ Proposed fix
First, extract the pending state from useCreateDocAccessRequest (line 55):
// Note: Depending on your React Query version, use `isLoading` (v4) or `isPending` (v5).
const { mutate: createRequest, isPending } = useCreateDocAccessRequest();Then, include it in the disabled prop:
<RequestAccessButton
onClick={() => createRequest({ docId: id, role: Role.EDITOR })}
- disabled={hasRequested}
+ disabled={hasRequested || isPending}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <RequestAccessButton | |
| onClick={() => createRequest({ docId: id, role: Role.EDITOR })} | |
| disabled={hasRequested} | |
| > | |
| <RequestAccessButton | |
| onClick={() => createRequest({ docId: id, role: Role.EDITOR })} | |
| disabled={hasRequested || isPending} | |
| > |
🤖 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
`@src/frontend/apps/impress/src/features/docs/doc-management/components/DocPage403.tsx`
around lines 130 - 133, Update the useCreateDocAccessRequest destructuring to
capture its mutation-in-progress state, then combine that state with
hasRequested in RequestAccessButton’s disabled condition so rapid clicks cannot
dispatch duplicate requests.
51b0d03 to
93bf4d8
Compare
2 participants
Purpose
This PR redesigns error and access-related pages (404, 500, 503, 401, 403, email confirmation) to match the new Figma specs, with consistent layout, typography, icons, and action links.
Figma : here
issue : 1561
newerrorpage.mp4
Proposal
ErrorPagecomponent (102×72 icon, 12px description, text-link actions)_errorpages to useErrorPageerrorActionStyles,errorDescriptionStyles) and reuseErrorPageon offline page