refactor(ui): centralize compact icon button sizing in EMCN - #7991
BillLeoutsakosvl346 wants to merge 16 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@greptileai @cubic-dev-ai Please review the current head; the previous scored reviews cover earlier commits. |
@BillLeoutsakosvl346 I’ve started reviewing the changes since the last completed review. I’ll post the results here when it’s done. |
|
@greptileai @cubic-dev-ai Please review the latest commit. The import-order directive now uses TSDoc while retaining the documented import order and passing Biome. |
@BillLeoutsakosvl346 I’ve started reviewing the changes since the last completed review. I’ll post the results here when it’s done. |
|
@cubic-dev-ai Please run a full review of this PR against its base branch on the current head, rather than an incremental review of only the latest commit. The latest incremental review reported no issues; this is the final review of the complete change. |
@BillLeoutsakosvl346 I have started the AI code review. It will take a few minutes to complete. |
Before / after visual comparisonRepresentative icon-button sizing combinations after replacing local size classes with shared sizes. Within each numbered tile: first control = before; second control = after. Rendered component fixtures with application CSS and local data, rather than authenticated product-page screenshots. These are representative visual examples, not end-to-end behavior coverage. The fixture uses a generic X icon to isolate button sizing; these are not screenshots of the actual caller icons. This comparison covers the sizing migration, not every message-action interaction. Current PR head checked at posting:
Additional theme/state comparisons
|
Co-authored-by: Bill Leoutsakos <billleoutsakos@Mac.localdomain>
0be585b to
13f85d8
Compare
|
@cubic-dev-ai review this PR |
@BillLeoutsakosvl346 I have started the AI code review. It will take a few minutes to complete. |
|
@greptile Please review the current head against staging, including the merged consolidation stack and conflict resolutions. |
|
@cubic-dev-ai review this PR. Please review the current head against staging, including the merged consolidation stack and conflict resolutions. |
@BillLeoutsakosvl346 I have started the AI code review. It will take a few minutes to complete. |
![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 286 files
Confidence score: 3/5
- In
apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-editor/preview-readonly-styles.ts, pointer-only blocking leaves the “Configure PII Types” modal reachable by keyboard in the read-only preview; make the preview subtree inert or disable the trigger.
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="apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-editor/preview-readonly-styles.ts">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-editor/preview-readonly-styles.ts:19">
P2: This blocks pointer input only; keyboard activation remains possible. The enabled “Configure PII Types” trigger can still open its configuration modal with Tab and Enter, so make the preview subtree inert or disable the trigger as well.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
|
@cubic-dev-ai review this PR. Please review the new head ef993ec against staging, including the visual parity and preview accessibility fixes. |
@BillLeoutsakosvl346 I have started the AI code review. It will take a few minutes to complete. |
|
Superseded by the current-head visual and code review guide, which adds the workflow panel tabs and final measured comparisons. |
There was a problem hiding this comment.
All reported issues were addressed across 284 files
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Re-trigger cubic
|
@cubic-dev-ai review this PR. Please review the current head 342c17a after the knowledge route import fix. |
@BillLeoutsakosvl346 I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-ai review this PR. Please review the current head 47a18f4; the panel-tab geometry issue is fixed and answered in its thread. |
@BillLeoutsakosvl346 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 284 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
Visual and code review guide · current head
|
| Light · 16px | Dark · 16px |
|---|---|
 |
 |
Enlarged 20px root text and open status-menu states
| Light · 20px | Dark · 20px |
|---|---|
 |
 |
| Status menu · before | Status menu · after |
|---|---|
 |
 |
 |
 |
What moved to shared APIs
Icon geometry. The caller keeps its icon and behavior; EMCN owns the exact 28px target. The same pattern covers table options and other repeated icon actions.
// Before
<Button size='sm' iconPadding='sm' className='size-7 shrink-0'>
<X className='size-[12px]' />
</Button>
// After
<Button size='sm' iconPadding='sm' iconSize='regular' className='shrink-0'>
<X className='size-[12px]' />
</Button>Workflow row actions. The repeated zero-padding treatment is now size='bare'. The same source icon, click handler and destructive color remain.
// Before
<Button variant='ghost-destructive' className='h-auto p-0'>…</Button>
// After
<Button variant='ghost-destructive' size='bare'>…</Button>Workflow panel tabs. The shared three-tab wrapper retains staging's Button size, radius, text size, border and state colors. The attempted Chip migration was removed after visual review.
// Before: repeated at Chat, Toolbar and Editor
<Button variant={active ? 'active' : 'ghost'}
className='h-[28px] rounded-md border py-[5px] text-[12.5px] …'>
{label}
</Button>
// After: same Button and classes, shared in PanelTabButton
<PanelTabButton tab='toolbar' active={active} onClick={onClick}>
Toolbar
</PanelTabButton>Selection menus. The duplicate ChipDropdown is gone. ChipSelect owns the trigger/menu geometry and keyboard selection. The staging conversation-mode picker retains its round, bare trigger, content-width menu and open-state tooltip behavior.
// Before
<ChipDropdown variant='ghost' shape='round' matchTriggerWidth={false}
options={options} value={value} onChange={setValue} />
// After
<ChipSelect variant='ghost' dropdownWidth='content' modal={false}
options={options} value={value} onChange={setValue} />Component-specific chrome. Canvas actions use CanvasControlButton; browser Back/Forward use BrowserToolbarButton; workflow field disclosures share a feature-owned row. Their focused before/after image reviews are here: canvas actions, browser toolbar, workflow disclosures, and table option actions. The old table-filter image in #8149 predates the later consolidation; the current status-filter comparison above reflects the final PR.
Review changes after the stack met staging
- Preserved Waleed’s newer chat composer, source cards, resource rows, tab strip and collaborator styling while resolving conflicts. The new staging
Buttonvariant andRowActionsexport stay intact. - Restored the exact staging inactive mic hover/color, admin pager, table “New row”, sidebar avatar/favicon, logs Download label, MCP Advanced toggle and public-auth description contrast after Cubic identified differences.
- The PII selector in read-only preview is now truly disabled, including keyboard activation, while its preview value remains visible. Tag removal controls now name the tag for screen readers. The attempted panel tab text override was removed with the Chip migration.
- The local same-package EMCN imports avoid a barrel dependency cycle; the 75 guarded tool-registry entry graphs now pass.
The source PRs contain the individual consumer diffs. This guide shows the repeated treatments and their visible states so reviewers can spend their time on the remaining behavior and intentional choices. Reports, image fixtures and measurements are external to the product branch.
This branch was previously deployed
1 participant
Review map
This is the last staging-facing parent of the icon-action and EMCN control stack. The merged work is grouped below so reviewers can follow one treatment at a time:
iconSize, responsive sizes, inline/bare sizes, round shape and muted focus ring replace repeated exact classes. Message actions intentionally use the shared 24px quiet treatment.ChipSelectmenu-selection API replaces the duplicate dropdown; segmented controls and ordinary auth/credential fields use their shared EMCN owners. Removed options were migrated at consumers.ChipSelectwith the same bare round trigger, content-width menu and tooltip visibility signal. The new stagingButtonsecondary variant andRowActionsexport are retained. Direct knowledge imports retain the lean route graphs.The consolidation has these intentional visible changes: multiple-selection menus use checkbox rows; segmented groups have zero gap; former large badges use medium sizing; table bulk actions use the adaptive dark fill; migrated ordinary fields use chip corners and surfaces; logs Clear All Filters becomes 30px instead of 32px; and knowledge-tag removal becomes a 20px accessible action instead of a 16px target. In this final refresh, 18 repeated zero-padding workflow icon actions became
Button size='bare'with the same final classes. Exact staging mic, pager, table-row, sidebar, public-auth, MCP and workflow-panel treatments were restored after visual review. The landing hero block title remains 17px, as on staging. Landing and docs styling is outside this product-system migration.Representative code changes
Icon geometry (same rendered classes):
Workflow row actions (same rendered classes):
Staging conversation-mode picker (retains its existing tooltip/menu behavior):
Other control owners:
CanvasControlButtonreplaces repeated canvas action classes;BrowserToolbarButtonkeeps the 30px browser treatment;ChipButtonGroupowns segmented controls. Source PRs above contain their focused implementation diffs.Validation and visual review
47a18f4940; all review threads are answered and resolved. Every required check passes, including the PostgreSQL integration job on rerun after an unchanged test timed out on its first attempt. The PR is mergeable against staging.No audit reports, fixture code, or screenshots are included in this product branch. This PR is for review only; it will not be merged automatically.