Skip to content

fix(ui): resolve MCP promo banner dismiss button overlap#581

Merged
shadowfax92 merged 1 commit intomainfrom
fix/ui-dimiss-for-mcp
Mar 26, 2026
Merged

fix(ui): resolve MCP promo banner dismiss button overlap#581
shadowfax92 merged 1 commit intomainfrom
fix/ui-dimiss-for-mcp

Conversation

@shadowfax92
Copy link
Copy Markdown
Contributor

Summary

  • Fix dismiss (X) button overlapping with "Set up" button text in the MCP promo banner
  • Move dismiss button from absolute positioning to inline flex child within the banner's flex layout
  • Remove unnecessary relative from container since no absolute-positioned children remain

Test plan

  • Open Settings → BrowserOS AI page
  • Verify MCP promo banner shows the X dismiss button properly spaced from the "Set up" button
  • Click X to dismiss the banner — verify it disappears
  • Click "Set up" — verify it navigates to /settings/mcp

Move dismiss button from absolute positioning to inline flex child,
preventing it from overlapping with the "Set up" button.
@shadowfax92
Copy link
Copy Markdown
Contributor Author

@greptileai review

@shadowfax92
Copy link
Copy Markdown
Contributor Author

@claude review

@github-actions github-actions bot added the fix label Mar 26, 2026
@claude
Copy link
Copy Markdown

claude bot commented Mar 26, 2026

Claude finished @shadowfax92's task in 40s —— View job


Code Review

  • Read changed file and diff
  • Analyze the fix
  • Post review feedback

LGTM. This is a clean, minimal fix for the layout issue.

What changed (McpPromoBanner.tsx):

  1. Line 20 — Removed relative from the container div. Since the dismiss button is no longer absolutely positioned, relative was only there to anchor it, so removing it is correct.

  2. Line 51 — Changed the dismiss <button> from absolute top-2 right-2 to shrink-0. This moves it into the natural flex flow alongside the "Set up" <Button>, so the two elements are now siblings in the flex row rather than overlapping via absolute positioning.

No issues found:

  • shrink-0 correctly prevents the X button from being squeezed on narrow widths, matching the pattern already used on the icon container (line 21) and "Set up" button (line 43).
  • The gap-4 on the container provides natural spacing between the "Set up" button and the X — no extra margin needed.
  • Functionality (dismiss logic, navigation) is unchanged.
  • No accessibility regressions — type="button" is still present.

Minor nit (optional): The X button has no aria-label. Adding aria-label="Dismiss" would improve screen reader accessibility, but this is pre-existing and out of scope for this fix.

Fix this →

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR fixes a layout bug in McpPromoBanner.tsx where the dismiss (X) button was absolutely positioned and visually overlapped the "Set up" button. The fix is minimal and targeted: the button is moved into the flex row as a shrink-0 inline child, and the now-unnecessary relative class is removed from the container.\n\n- Removes relative from the outer div — correct, since no children are absolutely positioned anymore.\n- Replaces absolute top-2 right-2 with shrink-0 on the dismiss button — places it naturally after the "Set up" button in the flex flow with gap-4 spacing.\n- No logic, state, or analytics changes — purely a layout correction.\n\nOne pre-existing observation (not introduced by this PR): the dismissed state is backed only by useState(false) and will reset on every page navigation, so the banner will reappear each time the user visits the AI-settings page. If persistent dismissal is desired, it would require storing the preference (e.g., via BrowserOS preferences). This is out of scope for this fix but worth tracking as a follow-up.

Confidence Score: 5/5

Safe to merge — single-file layout fix with no logic changes or regressions.

The change is a two-line CSS class swap that correctly resolves the described overlap bug. The before/after layout is easy to reason about: removing absolute top-2 right-2 and adding shrink-0 as a flex child places the dismiss button cleanly after the "Set up" button. No data, state, navigation, or analytics logic was touched.

No files require special attention.

Important Files Changed

Filename Overview
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/McpPromoBanner.tsx Moves X dismiss button from absolute positioning into the flex row as an inline shrink-0 child, fixing overlap with the "Set up" button; removes now-unnecessary relative from the container.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["McpPromoBanner renders"] --> B{"dismissed?"}
    B -- "true" --> C["return null"]
    B -- "false" --> D["Flex row: gap-4, items-center"]
    D --> E["Server icon (shrink-0)"]
    D --> F["Text content (flex-1, min-w-0)"]
    D --> G["Set up Button (shrink-0)"]
    D --> H["X dismiss button (shrink-0) moved inline"]
    H -- "onClick" --> I["setDismissed(true)"]
    I --> C
    G -- "onClick" --> J["track() + navigate('/settings/mcp')"]
Loading

Reviews (1): Last reviewed commit: "fix(ui): resolve MCP promo banner dismis..." | Re-trigger Greptile

@shadowfax92 shadowfax92 merged commit 085352a into main Mar 26, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant