Skip to content

fix(webhook-notif): modal positioning - #1771

Merged
icecrasher321 merged 1 commit into
stagingfrom
fix/webhook-notif-modal
Oct 30, 2025
Merged

fix(webhook-notif): modal positioning#1771
icecrasher321 merged 1 commit into
stagingfrom
fix/webhook-notif-modal

Conversation

@icecrasher321

@icecrasher321icecrasher321 commented Oct 30, 2025

Copy link
Copy Markdown
Collaborator

Summary

Webhook Notification modal positioning should not be relative.

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

vercelBot commented Oct 30, 2025

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentPreviewCommentsUpdated (UTC)
docsSkippedSkippedOct 30, 2025 9:28pm

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Removed the redundant relative class from the DialogContent component in the webhook notification modal. The DialogContent component already applies fixed positioning (as seen in apps/sim/components/ui/dialog.tsx:73), which positions the dialog absolutely relative to the viewport. Adding relative was conflicting with this positioning strategy and was unnecessary.

Key changes:

  • Removed relative from the className prop of DialogContent at line 508
  • Modal now correctly uses only the fixed positioning inherited from the base component
  • No functional changes to the modal's behavior or content

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The change is a minimal CSS class removal that fixes a positioning conflict. The relative class was redundant since the DialogContent component already uses fixed positioning. This is a straightforward bug fix with no logic changes or side effects.
  • No files require special attention

Important Files Changed

File Analysis

FilenameScoreOverview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/control-bar/components/webhook-settings/webhook-settings.tsx5/5Removed redundant relative class from DialogContent, as the component already uses fixed positioning

Sequence Diagram

sequenceDiagram
participant User
participant WebhookSettings
participant Dialog
participant DialogContent
User->>WebhookSettings: Opens webhook settings modal
WebhookSettings->>Dialog: open=true
Dialog->>DialogContent: Render with className
Note over DialogContent: Before: 'relative flex h-[70vh]...'<br/>After: 'flex h-[70vh]...'
DialogContent->>DialogContent: Apply fixed positioning<br/>(top-[50%] left-[50%]<br/>translate-x/y-[-50%])
Note over DialogContent: 'relative' class was redundant<br/>and conflicted with 'fixed'
DialogContent-->>User: Display correctly positioned modal
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@icecrasher321
icecrasher321 merged commit 3b901b3 into stagingOct 30, 2025
9 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/webhook-notif-modal branch November 7, 2025 20:18
waleedlatif1 pushed a commit that referenced this pull request Nov 12, 2025
@waleedlatif1waleedlatif1 mentioned this pull request Nov 12, 2025
10 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@icecrasher321