Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Remove custom h-[32px] from Restore/Save Changes buttons to use default emcn button size
  • Replace useEffect for clearing messages with event handler logic in onOpenChange

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
Copy link

vercel bot commented Jan 30, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jan 30, 2026 7:40pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

Removed custom h-[32px] height overrides from Restore/Save Changes buttons to use the EMCN component library's default size variants. Replaced useEffect anti-pattern with proper event handler logic - message clearing now happens directly in the onOpenChange callback when the modal opens, eliminating unnecessary effect synchronization.

Changes:

  • Removed h-[32px] className from two buttons on lines 699 and 707, allowing default EMCN button sizing (md variant with px-[8px] py-[6px])
  • Moved setErrorMessage(null) and setSuccessMessage(null) calls from a separate useEffect into the onOpenChange handler's opening branch
  • Deleted redundant useEffect that was watching the open prop to clear messages

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • Both changes follow best practices - removing custom size overrides aligns with component library patterns, and replacing the useEffect with event handler logic eliminates an anti-pattern while maintaining the same functionality
  • No files require special attention

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/invite-modal/invite-modal.tsx Removed custom button height overrides and replaced useEffect anti-pattern with event handler logic

Sequence Diagram

sequenceDiagram
    participant User
    participant Modal
    participant onOpenChange
    participant State
    
    User->>Modal: Opens modal (open=true)
    Modal->>onOpenChange: onOpenChange(true)
    onOpenChange->>State: setErrorMessage(null)
    onOpenChange->>State: setSuccessMessage(null)
    Note over State: Messages cleared on open
    
    User->>Modal: Closes modal (open=false)
    Modal->>onOpenChange: onOpenChange(false)
    onOpenChange->>State: resetState()
    Note over State: Full state reset on close
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

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.

@waleedlatif1 waleedlatif1 merged commit 6cf9841 into staging Jan 30, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/invite-modal branch January 30, 2026 20:14
Sign up for free to 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.

2 participants