Put the navbar tabs and the show page header on one 40px grid - #25236
Put the navbar tabs and the show page header on one 40px grid#25236FelixMalfait wants to merge 1 commit into
Conversation
The mode switcher gets the top and bottom rules from the Figma component and the 40px row height that goes with them. The page card header and the record identifier bar drop from 48px to 40px so the switcher's rules land on the card header's bottom border and on the tab strip's bottom border, in the main container and in the side panel alike. The drawer's top padding drops to 4px and its header no longer takes the section gap, which is what puts the switcher row where the card header ends.
|
🚀 Preview Environment Ready! Your preview environment is available at: https://mrna-knew-supporting-sealed.trycloudflare.com This environment will automatically shut down after 5 hours. |
🤖 PR Review
🛡️ Security Review✅ No high-severity vulnerabilities detected. 🚦 Auto-approve✅ Eligible for auto-approval — low-complexity, no high-blast-radius paths.
Automated pre-review — human approval still required. |
🟡 Quality review · 2 findings
High-level — One small, self-contained Linaria layout-alignment change; no data model, migration, flag, or public-surface concerns across product/architecture/build/deploy 💬 2 inline comments on the diff. Reviewed against the |
![twenty-ci-bot-public[bot]](https://avatars.githubusercontent.com/u/119600397?s=60&v=4){kind=small}
| } | ||
| `; | ||
|
|
||
| // 4px above the 32px header plus a 4px gap puts the mode switcher row exactly |
There was a problem hiding this comment.
🟡 Nit · Low-level · comments
Comment restates the pixel arithmetic of the padding/gap tokens below it
The prose hardcodes "4px", "32px" and "40px" that merely paraphrase the spacing tokens and will silently drift if any token changes. Delete it — the layout intent is not an external constraint the code cannot express.
| import { WorkspaceActivationStatus } from 'twenty-shared/workspace'; | ||
| import { PermissionFlagType } from '~/generated-metadata/graphql'; | ||
|
|
||
| // The row is as tall as the page card header row it sits next to, so its rules |
There was a problem hiding this comment.
🟡 Nit · Low-level · comments
Comment paraphrases the height/border rules it precedes
It describes what the border-top/border-bottom and height:spacing[10] already say rather than a constraint from outside the file. It fails the deletion test — remove it.
CoreTeamTwenty
left a comment
CoreTeamTwenty
left a comment
There was a problem hiding this comment.
✅ Auto-approved — cleared automated quality and security review and is low-complexity.
- Quality: passed — 2 nit(s)
- Security: passed
- Auto-approve: passed — complexity low
Automated approval — a human still merges, and this is re-evaluated on every push.
Greptile SummaryThe PR aligns navigation tabs, record headers, and desktop side-panel headers to a 40px grid.
Confidence Score: 4/5The mobile side-panel list-height calculation should be corrected before merging because it now reserves 40px for a top bar that remains 52px tall. The desktop height constant is reused in the mobile list calculation, so lowering it increases the mobile viewport overrun to 12px. Files Needing Attention: packages/twenty-front/src/modules/side-panel/constants/SidePanelTopBarHeight.ts and packages/twenty-front/src/modules/side-panel/components/SidePanelList.tsx Important Files Changed
Reviews (1): Last reviewed commit: "Put the navbar tabs and the show page he..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -1 +1 @@ | |||
| export const SIDE_PANEL_TOP_BAR_HEIGHT = 48; | |||
| export const SIDE_PANEL_TOP_BAR_HEIGHT = 40; | |||
There was a problem hiding this comment.
When a mobile side-panel list reaches its maximum height, SidePanelList subtracts this 40px desktop constant even though the rendered mobile top bar remains 52px tall, causing the list to extend 12px beyond its available viewport and clip final content or introduce unintended outer scrolling.
Knowledge Base Used: Client application shell and navigation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-front/src/modules/side-panel/constants/SidePanelTopBarHeight.ts">
<violation number="1" location="packages/twenty-front/src/modules/side-panel/constants/SidePanelTopBarHeight.ts:1">
P2: Keep the mobile list height calculation aligned with the 52px mobile top bar instead of applying this 40px desktop constant; otherwise the list can overflow the viewport and clip its final content.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -1 +1 @@ | |||
| export const SIDE_PANEL_TOP_BAR_HEIGHT = 48; | |||
| export const SIDE_PANEL_TOP_BAR_HEIGHT = 40; | |||
There was a problem hiding this comment.
P2: Keep the mobile list height calculation aligned with the 52px mobile top bar instead of applying this 40px desktop constant; otherwise the list can overflow the viewport and clip its final content.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/side-panel/constants/SidePanelTopBarHeight.ts, line 1:
<comment>Keep the mobile list height calculation aligned with the 52px mobile top bar instead of applying this 40px desktop constant; otherwise the list can overflow the viewport and clip its final content.</comment>
<file context>
@@ -1 +1 @@
-export const SIDE_PANEL_TOP_BAR_HEIGHT = 48;
+export const SIDE_PANEL_TOP_BAR_HEIGHT = 40;
</file context>
2 participants
The mode switcher gets the top and bottom rules from the Figma component and the 40px row height that goes with them. The page card header and the record identifier bar drop from 48px to 40px so the switcher's rules land on the card header's bottom border and on the tab strip's bottom border, in the main container and in the side panel alike.
The drawer's top padding drops to 4px and its header no longer takes the section gap, which is what puts the switcher row where the card header ends.