fix: MCP Server Modal Improvements

[deon-sanchez]

This pull request updates the modal system to allow custom sizing and improves the layout and usability of the AddMcpServerModal form. The main changes are the introduction of new height and width props for modals and several layout refinements for a more flexible and consistent user interface.

Modal Sizing Enhancements:

• Added optional height and width props to BaseModal, allowing consumers to specify custom dimensions for modals. The modal now applies these styles dynamically if provided. [1] [2] [3] [4] [5] [6]

AddMcpServerModal Layout Improvements:

• Updated AddMcpServerModal to use the new width="500px" and height="533px" props instead of the previous "x-small" size, providing more precise control over its dimensions.
• Refined the structure and layout of the modal's tabs and content areas for better alignment, spacing, and responsiveness. This includes changes to flex layouts, gap usage, and tab list styling. [1] [2] [3] [4]

Screen.Recording.2026-03-03.at.1.21.23.PM.mov

Summary by CodeRabbit

Release Notes

• UI/UX Improvements

  • Enhanced modal layouts with improved responsive design and flexible spacing.

• Refactor

  • Improved internal modal component structure for better maintainability and layout customization capabilities.

[coderabbitai]

Walkthrough

This pull request adds explicit width and height sizing properties to the BaseModal component and refactors the AddMcpServerModal layout to use flexible flex-based styling instead of fixed dimensions, enabling adaptive responsive layouts.

Changes

Cohort / File(s)	Summary
BaseModal Enhancement src/frontend/src/modals/baseModal/index.tsx	Added height and width string props to BaseModalProps. Implemented conditional inline style application with customHeight and customWidth derived values. Applied styles to both modal and non-modal dialog variants with fallback to default constraints when custom dimensions are not provided.
AddMcpServerModal Layout Refactoring src/frontend/src/modals/addMcpServerModal/index.tsx	Replaced fixed size attributes with explicit width/height values (500px/533px). Restructured Tabs and content containers to use full-height flex layouts with vertical stacking. Converted TabsList from grid to horizontal flex layout with equal-width TabsTrigger items. Updated inner content blocks to use responsive height utilities (h-full, flex-1) instead of fixed constraints.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 4

❌ Failed checks (1 error, 3 warnings)

Check name	Status	Explanation	Resolution
Test Coverage For New Implementations	❌ Error	PR introduces new height and width props to BaseModal and modifies AddMcpServerModal styling, but includes no test files to verify functionality.	Add test files for BaseModal and AddMcpServerModal to verify new props work correctly and styling changes don't introduce regressions.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Quality And Coverage	⚠️ Warning	PR introduces new height and width props to BaseModal and uses them in AddMcpServerModal, but provides zero test coverage for this functionality.	Add comprehensive test files for BaseModal and AddMcpServerModal verifying new props are correctly applied as styles, responsive behavior works on smaller viewports, and default sizing functions without custom props.
Test File Naming And Structure	⚠️ Warning	Pull request modifies critical components without adding corresponding test files despite establishing new public API props and project's established testing conventions.	Create test files for baseModal and addMcpServerModal components following project patterns, including tests for new height and width props with positive and negative scenarios.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Excessive Mock Usage Warning	✅ Passed	No test files are introduced, modified, or removed in this PR, making the excessive mock usage check not applicable.
Title check	✅ Passed	The title accurately describes the main change: custom sizing enhancements to the BaseModal component and layout improvements to the AddMcpServerModal, addressing the PR's primary objectives.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch le-465

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Frontend Unit Test Coverage Report

Coverage Summary

Lines	Statements	Branches	Functions
	23.24% (8228/35395)	16.02% (4458/27813)	15.92% (1185/7441)

Unit Test Results

Tests	Skipped	Failures	Errors	Time
2631	0 💤	0 ❌	0 🔥	43.919s ⏱️

[viktoravelino]

lgtm

[codecov]

Codecov Report

❌ Patch coverage is 20.83333% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.46%. Comparing base (14e3cf7) to head (2dc1a25).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...rc/frontend/src/modals/addMcpServerModal/index.tsx	0.00%	11 Missing ⚠️
src/frontend/src/modals/baseModal/index.tsx	50.00%	2 Missing and 3 partials ⚠️
...d/src/modals/baseModal/helpers/switch-case-size.ts	0.00%	3 Missing ⚠️

❌ Your patch status has failed because the patch coverage (20.83%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (42.37%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12017      +/-   ##
==========================================
+ Coverage   37.45%   37.46%   +0.01%     
==========================================
  Files        1616     1616              
  Lines       79014    79027      +13     
  Branches    11936    11943       +7     
==========================================
+ Hits        29593    29609      +16     
+ Misses      47766    47760       -6     
- Partials     1655     1658       +3     

Flag	Coverage Δ
backend	57.42% <ø> (+0.05%)	⬆️
frontend	20.83% <20.83%> (+<0.01%)	⬆️
lfx	42.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...d/src/modals/baseModal/helpers/switch-case-size.ts	10.66% <0.00%> (-0.45%)	⬇️
src/frontend/src/modals/baseModal/index.tsx	53.60% <50.00%> (-0.42%)	⬇️
...rc/frontend/src/modals/addMcpServerModal/index.tsx	0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

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)

src/frontend/src/modals/addMcpServerModal/index.tsx (1)

361-383: ⚠️ Potential issue | 🟠 Major

Content area can clip dynamic form fields without scroll.

Line 361 makes the tab body flex-1 but not scrollable, while parent wrappers are overflow-hidden. With growing STDIO/HTTP lists, lower fields can become inaccessible.

Proposed fix

-              <div
-                className="flex w-full flex-1 flex-col gap-2 border-y p-4"
+              <div
+                className="flex w-full flex-1 flex-col gap-2 overflow-y-auto border-y p-4"
                 id="global-variable-modal-inputs"
               >

Also applies to: 390-390, 442-442

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/frontend/src/modals/addMcpServerModal/index.tsx` around lines 361 - 383,
The JSON tab content can grow beyond its container because the inner div
(TabsContent value="JSON" -> the child div with class "flex flex-1 flex-col
gap-2 h-full") is set to flex-1 but not scrollable while ancestors use
overflow-hidden; update the JSON (and similar) tab content containers (the
TabsContent "JSON" block and the equivalent blocks referenced at the other
locations) to enable internal scrolling by adding an overflow auto behavior to
the scrollable pane (e.g., make the div with class "flex flex-1 flex-col gap-2
h-full" into a scrollable container) and ensure the Textarea and dynamic field
groups (id="global-variable-modal-inputs") can shrink/expand within that
scrollable area so long lists (STDIO/HTTP) remain accessible.

🧹 Nitpick comments (1)

src/frontend/src/modals/addMcpServerModal/index.tsx (1)

295-296: Fixed modal dimensions risk breaking on small screens.

Using hardcoded width="500px" and height="533px" can make this modal unusable on narrower viewports. This conflicts with the stated responsiveness improvements.

Proposed adjustment

-      width="500px"
-      height="533px"
+      width="min(500px, calc(100vw - 2rem))"
+      height="min(533px, calc(100dvh - 2rem))"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/frontend/src/modals/addMcpServerModal/index.tsx` around lines 295 - 296,
The modal in AddMcpServerModal is using hardcoded width="500px" and
height="533px", which breaks on small screens; update the modal sizing to be
responsive by replacing these fixed values with responsive sizing (e.g., use
width="100%" with a maxWidth or responsive array/obj values, and maxHeight or
responsive heights) or use the modal library's size prop so the modal caps at
500px on large viewports but shrinks on small ones; locate the JSX in
AddMcpServerModal (the element with width and height attributes) and change to
responsive properties (maxWidth, width="100%" or responsive tokens and
maxHeight/overflowY for scroll) to ensure usability on narrow viewports.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/frontend/src/modals/baseModal/index.tsx`:
- Around line 271-274: The code forces minWidth to the provided customWidth
which prevents the modal from shrinking; update the customStyle construction in
baseModal (variable customStyle built from customHeight/customWidth) to stop
setting minWidth when customWidth is provided (i.e., only apply width:
customWidth and remove minWidth: customWidth), or expose a separate prop (e.g.,
minWidth) if a non-shrinking minimum is required; modify the customStyle object
where customHeight/customWidth are spread so it no longer includes minWidth:
customWidth.

---

Outside diff comments:
In `@src/frontend/src/modals/addMcpServerModal/index.tsx`:
- Around line 361-383: The JSON tab content can grow beyond its container
because the inner div (TabsContent value="JSON" -> the child div with class
"flex flex-1 flex-col gap-2 h-full") is set to flex-1 but not scrollable while
ancestors use overflow-hidden; update the JSON (and similar) tab content
containers (the TabsContent "JSON" block and the equivalent blocks referenced at
the other locations) to enable internal scrolling by adding an overflow auto
behavior to the scrollable pane (e.g., make the div with class "flex flex-1
flex-col gap-2 h-full" into a scrollable container) and ensure the Textarea and
dynamic field groups (id="global-variable-modal-inputs") can shrink/expand
within that scrollable area so long lists (STDIO/HTTP) remain accessible.

---

Nitpick comments:
In `@src/frontend/src/modals/addMcpServerModal/index.tsx`:
- Around line 295-296: The modal in AddMcpServerModal is using hardcoded
width="500px" and height="533px", which breaks on small screens; update the
modal sizing to be responsive by replacing these fixed values with responsive
sizing (e.g., use width="100%" with a maxWidth or responsive array/obj values,
and maxHeight or responsive heights) or use the modal library's size prop so the
modal caps at 500px on large viewports but shrinks on small ones; locate the JSX
in AddMcpServerModal (the element with width and height attributes) and change
to responsive properties (maxWidth, width="100%" or responsive tokens and
maxHeight/overflowY for scroll) to ensure usability on narrow viewports.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2fc6ca8 and b37ab65.

📒 Files selected for processing (2)

• src/frontend/src/modals/addMcpServerModal/index.tsx
• src/frontend/src/modals/baseModal/index.tsx

[coderabbitai]

⚠️ Potential issue | 🟠 Major

minWidth should not be forced to custom width.

Line 273 sets minWidth equal to customWidth, which prevents shrinking on smaller viewports and can push dialogs off-screen. Keep width configurable, but let the container shrink when needed.

Proposed fix

   const customStyle: React.CSSProperties = {
     ...(customHeight ? { height: customHeight } : {}),
-    ...(customWidth ? { width: customWidth, minWidth: customWidth } : {}),
+    ...(customWidth ? { width: customWidth, maxWidth: "calc(100vw - 2rem)" } : {}),
   };

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const customStyle: React.CSSProperties = {
	...(customHeight ? { height: customHeight } : {}),
	...(customWidth ? { width: customWidth, minWidth: customWidth } : {}),
	};
	const customStyle: React.CSSProperties = {
	...(customHeight ? { height: customHeight } : {}),
	...(customWidth ? { width: customWidth, maxWidth: "calc(100vw - 2rem)" } : {}),
	};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/frontend/src/modals/baseModal/index.tsx` around lines 271 - 274, The code
forces minWidth to the provided customWidth which prevents the modal from
shrinking; update the customStyle construction in baseModal (variable
customStyle built from customHeight/customWidth) to stop setting minWidth when
customWidth is provided (i.e., only apply width: customWidth and remove
minWidth: customWidth), or expose a separate prop (e.g., minWidth) if a
non-shrinking minimum is required; modify the customStyle object where
customHeight/customWidth are spread so it no longer includes minWidth:
customWidth.

[olayinkaadelakun]

is there are reason why we are using both style className. Can we just use tailwind? Same with the divs below

[deon-sanchez]

valid concern, the styles was for adopting the custom width and height the classname was for tailwind, but i agree, we can simply the DOM by reomving style={}. will do

[deon-sanchez]

Screen.Recording.2026-03-04.at.9.13.23.AM.mov

fixed