style(frontend): resolve logo height, gate project defaults, and update back buttons responsively#602
Conversation
…nd update back buttons responsively
|
Thank you for contributing to Dembrane ECHO! Before we consider your Pull Request, we ask that you sign our Contributor License Agreement (CLA). This is only required for your first Pull Request. Please review the CLA, and sign it by adding your GitHub username to the contributors.yml file. Thanks! |
WalkthroughLegal-basis settings are now read-only for non-org-admins (fetches roles, derives isOrgAdmin, disables inputs and Save). Header/breadcrumb/sidebar/logo heights were standardized; settings header switched to a responsive flex layout; My Access card layout/imports adjusted and the “New organisation workspace” action removed. LGTM. ChangesOrganisation Admin Gating
UI Layout Height Standardization & Settings UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Speak like a true SF 100x engineer. LGTM. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🤖 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 `@echo/frontend/src/components/settings/LegalBasisSettingsCard.tsx`:
- Around line 113-120: The Alert in LegalBasisSettingsCard is using an off-brand
color prop (color="blue"); update the Alert within the LegalBasisSettingsCard
component (the JSX block that renders Alert when !isOrgAdmin) to use the brand
token by changing color="blue" to color="primary" or removing the color prop
entirely so it falls back to the theme default; ensure only the Alert element
inside the !isOrgAdmin conditional is modified.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ba8e78f-1fb3-4ccb-96d4-634e902f40a4
📒 Files selected for processing (5)
echo/frontend/src/components/common/Logo.tsxecho/frontend/src/components/settings/LegalBasisSettingsCard.tsxecho/frontend/src/features/sidebar/breadcrumbs/AppBreadcrumbs.tsxecho/frontend/src/features/sidebar/shell/SidebarHeader.tsxecho/frontend/src/routes/settings/UserSettingsRoute.tsx
| {!isOrgAdmin && ( | ||
| <Alert variant="light" color="blue" title={t`Read-only`}> | ||
| <Text size="sm"> | ||
| <Trans> | ||
| These settings are read-only. Only organisation administrators can modify project defaults. | ||
| </Trans> | ||
| </Text> | ||
| </Alert> |
There was a problem hiding this comment.
Use the brand color token on this alert.
color="blue" is off-brand in this repo. Swap it to color="primary" or omit the prop.
As per coding guidelines "Never pass color="blue" on Mantine components; use color="primary" instead or omit (primary is the theme default)".
🤖 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 `@echo/frontend/src/components/settings/LegalBasisSettingsCard.tsx` around
lines 113 - 120, The Alert in LegalBasisSettingsCard is using an off-brand color
prop (color="blue"); update the Alert within the LegalBasisSettingsCard
component (the JSX block that renders Alert when !isOrgAdmin) to use the brand
token by changing color="blue" to color="primary" or removing the color prop
entirely so it falls back to the theme default; ensure only the Alert element
inside the !isOrgAdmin conditional is modified.
…nd set badge text charcoal
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
echo/frontend/src/routes/settings/UserSettingsRoute.tsx (1)
69-74:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse Mantine layout primitives here and keep ActionIcon on brand.
Line 69/70 swaps to raw
<div>wrappers, and Line 73 setscolor="gray"onActionIcon; both drift from the UI guidelines for this surface.Proposed patch
- <div className="flex items-center"> - <div className="hidden md:flex w-[57px] shrink-0 items-center"> + <Group align="center" gap={0} className="flex"> + <Box className="hidden md:flex w-[57px] shrink-0 items-center"> <ActionIcon variant="subtle" - color="gray" + color="primary" onClick={() => navigate("..")} aria-label={t`Go back`} size={32} className="opacity-40 hover:opacity-100 transition-opacity" > <IconArrowLeft size={18} /> </ActionIcon> - </div> + </Box> <Title order={2}> <Trans>Settings</Trans> </Title> - </div> + </Group>As per coding guidelines: “Compose with Mantine primitives (
Stack,Group,ActionIcon)…” and “Allowed colors forButton/ActionIcon:primary… Never useblue,” with brand defaults centered onprimaryfor this UI system.🤖 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 `@echo/frontend/src/routes/settings/UserSettingsRoute.tsx` around lines 69 - 74, The header layout uses raw divs and a gray ActionIcon which violates our UI guidelines; update the wrapper divs in UserSettingsRoute to use Mantine primitives (e.g., Group or Stack) instead of the raw <div> around the icon and replace the ActionIcon color="gray" with the brand color (color="primary"), preserving the existing layout behavior (hidden on small screens, fixed width/shrink behavior) via Mantine props or sx styles and keep the onClick={() => navigate("..")} handler on the same ActionIcon.
🤖 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 `@echo/frontend/src/components/settings/MyAccessCard.tsx`:
- Line 149: In MyAccessCard.tsx replace the two hardcoded badge color attributes
c="`#2D2D2C`" (occurring in the component MyAccessCard around the badge/Icon
usages) with a Mantine theme token or CSS variable instead of a hex literal; use
useMantineTheme() (or the project’s theme token helper) to pick the appropriate
token (for example theme.colors.gray[9] or the matching design token/CSS var)
and update both occurrences (the c prop at the two spots) so the Badge/Icon
color follows theming rather than a hardcoded value.
---
Outside diff comments:
In `@echo/frontend/src/routes/settings/UserSettingsRoute.tsx`:
- Around line 69-74: The header layout uses raw divs and a gray ActionIcon which
violates our UI guidelines; update the wrapper divs in UserSettingsRoute to use
Mantine primitives (e.g., Group or Stack) instead of the raw <div> around the
icon and replace the ActionIcon color="gray" with the brand color
(color="primary"), preserving the existing layout behavior (hidden on small
screens, fixed width/shrink behavior) via Mantine props or sx styles and keep
the onClick={() => navigate("..")} handler on the same ActionIcon.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb6074b1-8f3e-4c3c-ae74-655f2c95c512
📒 Files selected for processing (2)
echo/frontend/src/components/settings/MyAccessCard.tsxecho/frontend/src/routes/settings/UserSettingsRoute.tsx
| size="xs" | ||
| variant="light" | ||
| color={roleColor(organisation.role)} | ||
| c="#2D2D2C" |
There was a problem hiding this comment.
Replace hardcoded badge text hex with theme token.
Line 149 and Line 192 hardcode #2D2D2C; use a theme color token/CSS variable so this stays consistent with theming and branding.
Proposed patch
- c="`#2D2D2C`"
+ c="var(--mantine-color-graphite-9)"
...
- c="`#2D2D2C`"
+ c="var(--mantine-color-graphite-9)"As per coding guidelines: “Use Mantine color tokens… instead of hardcoding hex colors in components” and “Don't hardcode hex colors in components.”
Also applies to: 192-192
🤖 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 `@echo/frontend/src/components/settings/MyAccessCard.tsx` at line 149, In
MyAccessCard.tsx replace the two hardcoded badge color attributes c="`#2D2D2C`"
(occurring in the component MyAccessCard around the badge/Icon usages) with a
Mantine theme token or CSS variable instead of a hex literal; use
useMantineTheme() (or the project’s theme token helper) to pick the appropriate
token (for example theme.colors.gray[9] or the matching design token/CSS var)
and update both occurrences (the c prop at the two spots) so the Badge/Icon
color follows theming rather than a hardcoded value.
2 participants
Page Feedback Adjustments
32pxand set header (SidebarHeader) andAppBreadcrumbsheight to57px. Applied mathematical even padding (12.5px) on the top, bottom, and left of the logo, and centered breadcrumbs vertically.LegalBasisSettingsCardto query cached workspaces data and restrict editing to organisation administrators (owner/admin). Added a read-onlyAlertbanner and disabled controls for non-admin users.UserSettingsRouteto wrap the back button in ahidden md:flex w-[57px]margin container. Hover opacity effects (opacity-40 hover:opacity-100) and a reserved32pxsize are applied to theActionIcon.MyAccessCard.tsxas requested.MyAccessCard.tsx.UserSettingsRoute.tsx.#2D2D2Ctext for consistent contrast.Confidence: High. Fully aligned with the page feedback and responsive design guidelines.
Summary by CodeRabbit
Release Notes
New Features
Style