Restore primary icon buttons on pinned actions and group the panel footer - #24221
Restore primary icon buttons on pinned actions and group the panel footer#24221FelixMalfait wants to merge 5 commits into
Conversation
…oter Felix's earlier pass turned the record page and side panel pinned icon actions into borderless tertiary buttons; Thomas's Figma keeps them as bordered primary icon buttons, with labeled actions as primary buttons. - CommandMenuButton renders icon-only pinned items as IconButton primary at every container (record header chevrons, trash, favorite, footer) - the options dots trigger, the side panel close button and the compose footer icons follow the same variant; the panel expand stays tertiary so the close button reads as the main affordance - the record side panel footer now renders the options menu inside the right-aligned cluster, leftmost, instead of stranding it across the stretch of the measurement wrapper; widget footers keep their layout
|
🚀 Preview Environment Ready! Your preview environment is available at: https://decimal-leo-relate-breeding.trycloudflare.com This environment will automatically shut down after 5 hours. |
There was a problem hiding this comment.
All reported issues were addressed across 7 files
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Greptile SummaryRestores bordered primary styling across pinned actions, shared options triggers, side-panel controls, and compose-email footer buttons. It also moves the record options menu into the right-aligned pinned-action cluster, but the cluster’s width calculation does not reserve space for the newly embedded action.
Confidence Score: 4/5The PR should not merge until the footer capacity calculation reserves space for the leading options action, otherwise pinned actions can become inaccessible at constrained widths. The side-panel footer now embeds an options button inside the measured cluster while continuing to allocate pinned actions against the entire width, so an action may be clipped without being moved into the overflow menu. Files Needing Attention: packages/twenty-front/src/modules/command-menu-item/display/components/PinnedCommandMenuItemButtons.tsx Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Footer["Side-panel footer width"] --> Measure["Measure full action container"]
Measure --> Capacity["Select pinned items using full width"]
Options["Leading options action + outer gap"] --> Cluster["Rendered action cluster"]
Capacity --> Cluster
Cluster --> Clip["Last pinned action may be clipped"]
Capacity --> Overflow["Clipped action omitted from overflow list"]
Reviews (1): Last reviewed commit: "Restore primary icon buttons on pinned a..." | Re-trigger Greptile |
| <NodeDimension onDimensionChange={onContainerDimensionChange}> | ||
| <StyledContainer> | ||
| {leadingAction} | ||
| <StyledItemsContainer shouldReverse={!isSidePanelFooter}> |
There was a problem hiding this comment.
Leading action breaks width allocation
When pinned actions nearly fill a side-panel footer, the layout allocates them against the full measured width without reserving space for leadingAction or its gap. The final inline action is then clipped by the items container while remaining excluded from the overflow menu, making it inaccessible.
🟡 Standard review · 2 findings
High-level — Coherent low-risk frontend restyle (primary button variants) plus a leading-action width-reservation fix; no backend, migration, or feature-flag surface 💬 2 inline comments on the diff. Reviewed against the |
🔍 Visual Regression Review —
|
| Story | Verdict | Confidence | Explained by | |
|---|---|---|---|---|
| 🔴 | modules-commandmenu-commandmenu--limited-permissions |
regression | 97% | The HEAD renders an 'Unexpected Application Error!' (AggregateError in BUILDING_… |
1 item(s) to double-check (uncertain / low confidence)
| Story | Verdict | Confidence | Explained by | |
|---|---|---|---|---|
| 🟡 | modules-settings-playground-graphqlplayground--default |
uncertain | 72% | No file changed by this PR relates to the GraphQL Playground component. The shif… |
Changed stories
| Story | Diff % |
|---|---|
| modules-commandmenu-commandmenu--limited-permissions | 53% |
| modules-settings-playground-graphqlplayground--default | 3% |
| modules-sidepanel-sidepaneltopbar--root-command-menu | 0% |
| modules-commandmenu-commandmenu--matching-navigate | 0% |
| modules-commandmenu-commandmenu--matching-navigate-shortcuts | 0% |
| modules-commandmenu-commandmenu--no-results-search-fallback | 0% |
| modules-commandmenu-commandmenu--default-without-search | 0% |
| modules-commandmenu-commandmenu--sub-page-navigation | 0% |
| modules-commandmenu-recordpagesidepanelcommandmenudropdown--default | 0% |
| modules-commandmenu-recordpagesidepanelcommandmenudropdown--with-button-clicks | 0% |
| modules-workflow-actions-filter-workfloweditactionfilter--default | 0% |
| modules-workflow-actions-deleterecord-editaction--default | 0% |
| modules-workflow-actions-form-workfloweditactionformbuilder--default | 0% |
| modules-workflow-actions-form-workfloweditactionformbuilder--delete-fields | 0% |
| modules-sidepanel-sidepaneltopbar--subpage | 0% |
| modules-workflow-actions-form-workfloweditactionformbuilder--empty-form | 0% |
View run details · advisory mode
🔍 Automated Pre-Review✅ No issues detected - This PR is ready for human review. Automated pre-review — human approval still required. |
| export const PinnedCommandMenuItemButtons = ({ | ||
| leadingAction, | ||
| }: { | ||
| // Rendered inside the right-aligned cluster, before the pinned buttons, so |
There was a problem hiding this comment.
🟡 Nit · Low-level · comments
Multi-line comment on the leadingAction prop describes internal layout mechanics, not an external constraint
The three-line comment explains where the prop renders and why, which is internal layout reasoning rather than an outside-the-file constraint, so it fails the kept-comment exception. Remove it.
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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/command-menu-item/display/components/PinnedCommandMenuItemButtons.tsx">
<violation number="1" location="packages/twenty-front/src/modules/command-menu-item/display/components/PinnedCommandMenuItemButtons.tsx:131">
P3: When `leadingAction` is undefined, the `NodeDimension` is not rendered, so `onLeadingActionDimensionChange` never runs and `leadingActionWidth` keeps its last measured value in `commandMenuPinnedInlineLayoutFamilyState`. The fit calculation then keeps reserving space for a leading action that no longer exists, pushing pinned items into overflow. Reset `leadingActionWidth` to 0 whenever `leadingAction` is absent (e.g. run the callback with width 0 on the undefined path) so the reserved slot tracks the rendered leading action.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| // Pinned items are buttons in the footer, so the dropdown only repeats the | ||
| // ones the footer could not fit. | ||
| // ones the footer could not fit. This dropdown is itself the footer's |
There was a problem hiding this comment.
🟡 Nit · Low-level · comments
Added comment sentence restates the hasLeadingAction: true argument directly below it
The new "This dropdown is itself the footer's leading action, so the fit keeps its width reserved" line paraphrases the hasLeadingAction: true prop under it, so it fails the deletion test. Drop the added sentence.
|
Alternative approach in #24235 for comparison: the footer measures its actions row and provides the width via context, the pinned buttons shrink to fit, and the options menu is a plain first action. No Generated by Claude Code |
…24235) Alternative to #24221 for comparison, per the discussion about `leadingAction`. Same design fixes (identical button variant changes, identical visual result, verified pixel-identical in the running app), different footer layout mechanism. Merge one or the other, not both. ## The difference #24221 keeps the pinned buttons component stretching over the footer's free space to measure it, so the options menu has to be injected inside that stretchy cluster through a `leadingAction` slot, and its runtime-measured width is reserved through the shared layout state. Here the ownership flips: - `SidePanelFooter` measures its actions row once and provides the width through a small generic context (`SidePanelFooterWidthContext`). - `PinnedCommandMenuItemButtons` accepts an optional `containerWidth`. When provided it shrinks to fit and renders just the buttons row; no stretching, no self-measurement. Header surfaces pass nothing and keep the existing self-measuring behavior. - The options menu is a plain first action again, grouped by the footer's existing `justify-content: flex-end`. - The record page reserves the options menu footprint with a constant (`SIDE_PANEL_FOOTER_OPTIONS_RESERVED_WIDTH = 32`, a 24px small icon button plus the 8px footer gap) when deriving the width it hands to the buttons and to the overflow dropdown. ## Tradeoffs vs #24221 - Removed: the `leadingAction` ReactNode slot, the nested measuring node around it, and the width-reservation plumbing in the shared state and fit util (`leadingActionWidth`, `hasLeadingAction`). The fit util is untouched from main. - Added: a footer width context (generic, reusable by any adaptive footer action) and one constant. - The constant is the cost: if the options button ever changes size, it must be updated by hand, while #24221 tracks it automatically by measuring. It degrades gracefully (a few px of early overflow or clipping) rather than breaking. ## Design fixes carried over unchanged from #24221 - Icon-only pinned actions render as bordered primary icon buttons again (`CommandMenuButton`). - Options trigger and side panel close button are primary; expand stays tertiary. - Footer actions grouped bottom right with the options menu leftmost of the group, matching [the Figma frame](https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=110786-425955). - Widget footer buttons and compose email discard/attach are primary. --- _Generated by [Claude Code](https://claude.ai/code/session_01H4XJwMCrTgaN6e2NyLFrAK)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/24235?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
Fixes the design regressions Thomas flagged on record pages and the side panel, matching this Figma frame.
What was wrong
Changes
CommandMenuButton: icon-only pinned actions are back toIconButton variant="primary"(blue accent still applies to primary CTAs). This is the root cause fix, it covers the record page header and the side panel footer since every pinned action renders through this component.OptionsDropdownMenu: the shared options trigger is now primary (record side panel, workflow step footer, widget settings footer).SidePanelTopBar: the close button is now primary; the expand button stays tertiary so the close action stands out.PinnedCommandMenuItemButtonstakes aleadingActionrendered inside its right-aligned cluster, because its width-measurement wrapper stretches over the free space and pushed any separate sibling to the opposite edge.variant="primary", keeping the blue accent for the CTA only, and the compose email discard/attach icon buttons are primary instead of secondary.Verified
Checked against the Figma reference in the running app: record header shows all bordered buttons, footer actions are clustered bottom-right with the options menu first, top bar shows lighter expand next to a bordered close.
Generated by Claude Code