fix(modals): center modals in visible content area accounting for sidebar and panel by waleedlatif1 · Pull Request #3933 · simstudioai/sim

waleedlatif1 · 2026-04-04T02:01:21Z

Summary

Command K and all emcn modals now center in the visible content area (between sidebar and panel) instead of the full viewport
Fixed `--sidebar-width` CSS default to `0px` so non-workspace pages (login, landing) keep modals viewport-centered
Blocking script now always explicitly sets `--sidebar-width` on workspace pages, covering new users and error cases
Search modal accounts for the right panel on workflow pages using `isOnWorkflowPage` prop
emcn `ModalContent` uses `usePathname` to detect workflow pages and apply the panel-aware formula
Removed horizontal slide animation from emcn Modal (was tied to hardcoded `left: 50%`)

Type of Change

Bug fix

Testing

Tested manually

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…ebar and panel

cursor · 2026-04-04T02:01:28Z

PR Summary

Medium Risk
Changes global CSS variables/z-index and the pre-hydration layout script that affects every workspace page, so regressions could shift UI layering/positioning across the app. The rest is contained to modal/search UI behavior and analytics instrumentation.

Overview
Modals and Command-K search are now centered within the visible workspace content area by positioning them relative to --sidebar-width (and --panel-width on workflow pages) instead of hardcoding left: 50%.

The workspace pre-hydration script now always sets a --sidebar-width fallback (248px) on workspace routes, while the global CSS default for --sidebar-width is 0px to keep non-workspace pages centered.

The Command-K search modal is refactored into group components, adds new result categories (tables, files, knowledgeBases), and emits PostHog search_result_selected events on selection. Global styling also introduces a shared z-index/shadow scale and switches scrollbar colors to use theme variables.

^{Reviewed by Cursor Bugbot for commit 147c89d. Configure here.}

vercel · 2026-04-04T02:01:30Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Apr 4, 2026 2:01am

greptile-apps · 2026-04-04T02:05:04Z

Greptile Summary

This PR fixes modal centering across the workspace by replacing the previous viewport-based centering with a formula that accounts for the sidebar (always) and the right panel (workflow pages only). It does this through four coordinated changes: defaulting --sidebar-width to 0px in globals.css so non-workspace pages stay viewport-centered, adding an always-executed --sidebar-width setter in the blocking layout script for new users and error cases, wiring the same two-formula approach into the SearchModal via its isOnWorkflowPage prop, and baking identical logic into the shared ModalContent via usePathname.

Key changes:

--sidebar-width CSS default changed to 0px; blocking script now always explicitly sets it on workspace pages
Centering formula: non-workflow workspace pages → calc(var(--sidebar-width) / 2 + 50%), workflow pages → calc(50% + (var(--sidebar-width) - var(--panel-width)) / 2) — the math is correct in both cases
Horizontal slide animation removed from ModalContent since it was anchored to a fixed left: 50%
ModalContent now uses usePathname internally to detect workflow pages, coupling Next.js routing knowledge to the shared emcn component; SearchModal uses a prop instead, making the two approaches inconsistent

Confidence Score: 5/5

Safe to merge — centering math is correct, all edge cases in the blocking script are handled, and the only remaining findings are style/design suggestions.

All identified issues are P2: a custom rule flag on the globals.css edit (which is justified by necessity) and a design concern about embedding Next.js routing logic inside the shared emcn component. No correctness bugs, data integrity risks, or security issues exist.

apps/sim/components/emcn/components/modal/modal.tsx — contains the usePathname//w/ coupling that may need revisiting if the URL structure evolves.

Important Files Changed

Filename	Overview
apps/sim/app/_styles/globals.css	Changes `--sidebar-width` default to `0px` so non-workspace pages keep modals viewport-centered; necessary but violates the avoid-globals.css-edits custom rule.
apps/sim/app/layout.tsx	Blocking script now always explicitly sets `--sidebar-width` on workspace pages (including new-user fallback and catch branches), correctly avoiding any flash of the wrong centering position.
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.tsx	Replaces the static `left-1/2` Tailwind class with a dynamic inline `style.left` that accounts for both sidebar and (on workflow pages) the right panel; centering math is correct.
apps/sim/components/emcn/components/modal/modal.tsx	Introduces `usePathname` and a hard-coded `/w/` path check inside the shared emcn `ModalContent` to apply the panel-aware centering formula; couples Next.js routing logic to a generic component.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Page loads] --> B{pathname includes\n/workspace/?}
    B -- No --> C[CSS default:\n--sidebar-width = 0px]
    B -- Yes --> D[Blocking script runs]
    D --> E{localStorage\nsidebar-state?}
    E -- Yes, collapsed --> F[--sidebar-width = 51px]
    E -- Yes, expanded --> G[--sidebar-width = stored width]
    E -- No / error --> H[--sidebar-width = 248px]
    C & F & G & H --> I[Modal opens]
    I --> J{Which modal?}
    J -- SearchModal --> K{isOnWorkflowPage prop}
    J -- ModalContent emcn --> L{usePathname\nincludes /w/?}
    K -- Yes --> M["left: calc(50% + (sidebar - panel) / 2)"]
    K -- No --> N["left: calc(sidebar / 2 + 50%)"]
    L -- Yes --> M
    L -- No --> N
    M & N --> O[translate-x: -50%\nModal visually centered\nin available area]

_{Reviews (1): Last reviewed commit: "fix(modals): center modals in visible co..." | Re-trigger Greptile}

greptile-apps · 2026-04-04T02:05:08Z

apps/sim/components/emcn/components/modal/modal.tsx

+  const pathname = usePathname()
+  const isWorkflowPage = pathname?.includes('/w/') ?? false


Application-specific routing logic inside a shared emcn component

usePathname and the hard-coded /w/ path string embed knowledge of the Sim URL structure into ModalContent, a component that is meant to be a generic, reusable primitive. If the workflow URL segment ever changes (e.g. renamed from /w/ to /workflow/), this condition silently breaks centering for all emcn modals without any compile-time signal.

A cleaner approach is to accept an explicit prop (mirroring what SearchModal already does with isOnWorkflowPage) and let the call site decide the layout context:

export interface ModalContentProps extends React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> { showClose?: boolean size?: ModalSize /** Pass true on pages that have both a sidebar and a right panel (e.g. workflow canvas). */ hasRightPanel?: boolean }

Then replace the usePathname call with:

const { className, children, showClose = true, size = 'md', hasRightPanel = false, style, ...props } = ...

and use hasRightPanel in the style object instead of isWorkflowPage. This removes the Next.js and URL-structure dependency from the shared component entirely.

greptile-apps · 2026-04-04T02:05:09Z

apps/sim/app/_styles/globals.css

 */
 :root {
-  --sidebar-width: 248px; /* SIDEBAR_WIDTH.DEFAULT */
+  --sidebar-width: 0px; /* 0 outside workspace; blocking script sets actual value on workspace pages */


Editing globals.css — custom rule requires avoiding this unless absolutely necessary

The project rule is to avoid editing globals.css and move style changes to local component files instead. The --sidebar-width: 0px default is needed so modals on non-workspace pages don't shift, which is a valid reason, but it's worth confirming there is no alternative (e.g. declaring a scoped :root override in the workspace layout) before landing a change here.

If the edit is indeed the only clean option, the inline comment already added (/* 0 outside workspace; blocking script sets actual value on workspace pages */) documents intent well.

Rule Used: Avoid editing the globals.css file unless absolute... (source)

Learnt From
simstudioai/sim#367

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 147c89d. Configure here.}

cursor · 2026-04-04T02:07:19Z

apps/sim/components/emcn/components/modal/modal.tsx

        ref={ref}
        className={cn(
-          'fixed inset-0 z-[500] bg-black/10 backdrop-blur-[2px]',
+          'fixed inset-0 z-[var(--z-modal)] bg-black/10 backdrop-blur-[2px]',


Overlay and SModal content z-index now mismatched

Medium Severity

ModalOverlay was changed from z-[500] to z-[var(--z-modal)] (200), but SModalContent in s-modal.tsx still uses z-[500] and imports this same ModalOverlay. This creates a z-index gap: the overlay sits at 200 while the sidebar-modal content sits at 500. Elements using --z-popover (300) or --z-tooltip (400) can now visually render between the overlay and the SModal content, breaking the expected stacking order where the overlay dims everything beneath the modal.

Additional Locations (1)

apps/sim/components/emcn/components/modal/modal.tsx#L164-L165

^{Reviewed by Cursor Bugbot for commit 147c89d. Configure here.}

fix(modals): center modals in visible content area accounting for sid…

147c89d

…ebar and panel

waleedlatif1 closed this Apr 4, 2026

waleedlatif1 deleted the fix/modal-centering branch April 4, 2026 02:02

greptile-apps bot reviewed Apr 4, 2026

View reviewed changes

cursor bot reviewed Apr 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(modals): center modals in visible content area accounting for sidebar and panel#3933

fix(modals): center modals in visible content area accounting for sidebar and panel#3933
waleedlatif1 wants to merge 1 commit intostagingfrom
fix/modal-centering

waleedlatif1 commented Apr 4, 2026

Uh oh!

cursor bot commented Apr 4, 2026 •

edited

Loading

Uh oh!

vercel bot commented Apr 4, 2026

Uh oh!

greptile-apps bot commented Apr 4, 2026

Uh oh!

greptile-apps bot Apr 4, 2026

Uh oh!

greptile-apps bot Apr 4, 2026

Uh oh!

cursor bot left a comment

Uh oh!

cursor bot Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		const pathname = usePathname()
		const isWorkflowPage = pathname?.includes('/w/') ?? false

Conversation

waleedlatif1 commented Apr 4, 2026

Summary

Type of Change

Testing

Checklist

Uh oh!

cursor bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

vercel bot commented Apr 4, 2026

Uh oh!

greptile-apps bot commented Apr 4, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Apr 4, 2026

Choose a reason for hiding this comment

Overlay and SModal content z-index now mismatched

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cursor bot commented Apr 4, 2026 •

edited

Loading