fix(modals): center modals in visible content area and remove open/close animation

[waleedlatif1]

Summary

• Center all modals in the visible content area by accounting for sidebar width (and panel width on workflow pages) — previously modals were centered in the full viewport, so they appeared off-center when the sidebar was open
• Change `--sidebar-width` CSS default to `0px` so non-workspace pages (login, etc.) get proper viewport centering; blocking script always sets the real value on workspace pages
• Remove open/close slide animation from modal content — it was an existing animation that became visually jarring once the horizontal slide was removed as part of the centering fix

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)

[vercel]

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 3:04am

[cursor]

PR Summary

Low Risk
Low risk UI-only positioning changes; main risk is mis-centering/overlap on edge viewport sizes or routes where sidebar/panel assumptions differ.

Overview
Modals and the workspace search dialog are now horizontally centered within the visible content area by offsetting left based on --sidebar-width (and additionally --panel-width on /w/ workflow routes).

Non-workspace pages now default --sidebar-width to 0px to avoid off-center dialogs, while the pre-hydration script in layout.tsx explicitly sets a 248px fallback when sidebar state is missing/invalid on workspace routes.

Removes the modal content slide-in/out animation classes so open/close transitions no longer include the previous sliding motion.

^{Reviewed by Cursor Bugbot for commit 0f471fb. Configure here.}

[greptile-apps]

Greptile Summary

This PR fixes modal centering by accounting for the sidebar (and panel on workflow pages) when computing the horizontal left position, and removes the open/close slide animation from modal content. The changes are coherent and the math for the centering formulae is correct.

Key changes:

• globals.css: --sidebar-width default changed from 248px → 0px so non-workspace pages (login, etc.) get a true 50% center; workspace pages rely entirely on the inline blocking script
• layout.tsx: Blocking script now explicitly sets --sidebar-width in all branches (stored state present, stored state absent, error) instead of falling back silently to the old CSS default
• modal.tsx: ModalContent uses usePathname() to detect workflow pages and shifts left to account for both the sidebar and the right panel; slide + fade animation classes are removed from the content element
• search-modal.tsx: Same centering formula applied via the existing isOnWorkflowPage prop — no coupling issue here

Confidence Score: 5/5

Safe to merge — the centering logic is correct and all remaining findings are P2 style concerns.

All three comments are P2 (style/architecture suggestions). No P0 or P1 defects were found. The core centering math is correct for both page types, the blocking script improvements are sound, and the removal of animations is intentional. Score remains 5 because P2 findings do not block merge.

apps/sim/components/emcn/components/modal/modal.tsx — architectural coupling via usePathname worth revisiting if ModalContent is ever used outside the Next.js app context.

Important Files Changed

Filename	Overview
apps/sim/components/emcn/components/modal/modal.tsx	Removes slide animation classes and introduces usePathname-based centering logic; architectural concern around coupling routing to a generic EMCN component.
apps/sim/app/_styles/globals.css	Changes --sidebar-width default from 248px to 0px so non-workspace pages center correctly; violates project rule against editing globals.css but appears justified.
apps/sim/app/layout.tsx	Blocking script hardened to always set --sidebar-width explicitly on workspace pages (previously relied on CSS default); all branches now covered including missing stored state and errors.
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.tsx	Applies the same centering formula as modal.tsx for workflow vs non-workflow pages via the existing isOnWorkflowPage prop — correct prop-based approach, no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Page Load - Blocking Script] --> B{Is /workspace/ path?}
    B -- No --> C[Exit early\n--sidebar-width stays 0px]
    B -- Yes --> D{localStorage sidebar-state exists?}
    D -- No --> E[Set --sidebar-width = 248px default]
    D -- Yes --> F{isCollapsed?}
    F -- Yes --> G[Set --sidebar-width = 51px]
    F -- No --> H{stored width in valid range?}
    H -- In range --> I[Set --sidebar-width = stored px]
    H -- Too wide --> J[Set --sidebar-width = 30vw max]
    H -- Too narrow / missing --> K[Set --sidebar-width = 248px default]
    D -- Error --> L[Set --sidebar-width = 248px default]

    subgraph Modal Centering
        M{ModalContent rendered}
        M --> N{usePathname includes /w/?}
        N -- Yes workflow page --> O["left = calc(50% + (sidebar-width - panel-width) / 2)"]
        N -- No other page --> P["left = calc(sidebar-width / 2 + 50%)"]
    end

Loading

Comments Outside Diff (2)

1. apps/sim/components/emcn/components/modal/modal.tsx, line 142-143 (link)

   Routing concern in generic EMCN component

   ModalContent is a generic, reusable EMCN component, but it now calls usePathname() internally to derive layout offsets based on the current route. This couples the component library to the application's specific routing structure (/w/ routes).

   Contrast this with search-modal.tsx, which correctly receives isOnWorkflowPage as a prop — letting the parent decide the context. The two approaches are now architecturally inconsistent.

   A cleaner approach is to expose a prop on ModalContent that lets callers inject the appropriate left value, or to use a CSS custom property set at the layout level:

   // Option A: explicit prop
   interface ModalContentProps extends ... {
     leftOffset?: string   // e.g. 'calc(50% + (var(--sidebar-width) - var(--panel-width)) / 2)'
   }
   
   // Option B: CSS variable set in the workspace layout
   // Callers already can pass `style` to override — document that pattern instead

   Additionally, calling usePathname() inside every mounted ModalContent instance means every navigation event will trigger a re-render of every open modal to recompute isWorkflowPage, even when nothing relevant changed.

2. apps/sim/app/_styles/globals.css, line 13 (link)

   Edit to globals.css (project rule)

   The project rule is to avoid editing globals.css unless absolutely necessary, and to prefer local component-level styles. This change does appear necessary (it resets the CSS default so non-workspace pages get correct 50% centering), but it's worth acknowledging.

   If a future refactor moves the sidebar-width variable initialisation to a layout-level CSS module or inline style rather than globals.css, this default would no longer be needed here.

   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!}

_{Reviews (1): Last reviewed commit: "chore(modals): resolve merge conflict wi..." | Re-trigger Greptile}

[greptile-apps]

duration-200 is orphaned without animation classes

duration-200 remains in the base class string, but the animate-in/animate-out classes (previously in ANIMATION_CLASSES and CONTENT_ANIMATION_CLASSES) have been removed. duration-200 controls the duration of CSS animations/transitions, so without an active animation or transition to pair with it, this class has no effect on the content element.

If the intent is to keep an animation duration for future use, a comment to that effect would help. Otherwise it can be removed to avoid confusion.

Suggested change

	ref={ref}
	className={cn(
	ANIMATION_CLASSES,
	CONTENT_ANIMATION_CLASSES,
	'fixed top-[50%] z-[var(--z-modal)] flex max-h-[84vh] translate-x-[-50%] translate-y-[-50%] flex-col overflow-hidden rounded-xl bg-[var(--bg)] text-small ring-1 ring-foreground/10 duration-200',
	MODAL_SIZES[size],
	className
	'fixed top-[50%] z-[var(--z-modal)] flex max-h-[84vh] translate-x-[-50%] translate-y-[-50%] flex-col overflow-hidden rounded-xl bg-[var(--bg)] text-small ring-1 ring-foreground/10',