-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix: modals, settings, panel #2187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile OverviewGreptile SummaryThis PR improves the settings modal UI/UX, fixes panel behavior, and updates various modal components. The main changes include adding an MCP server detail view with navigation, fixing SSO tab visibility logic for self-hosted environments, improving the name editing input auto-sizing, removing unwanted automatic focus/tab-switching behavior in the panel, and updating styling consistency across skeleton loaders and modal components. Key changes:
Issues found:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant SettingsModal
participant MCPComponent
participant ServerListItem
participant MCPDetailView
participant API
Note over User,API: MCP Server Detail View Flow
User->>SettingsModal: Open Settings Modal
SettingsModal->>MCPComponent: Render MCP tab
MCPComponent->>API: Fetch MCP servers
API-->>MCPComponent: Return servers list
MCPComponent->>ServerListItem: Render server list items
User->>ServerListItem: Click "Details" button
ServerListItem->>MCPComponent: Call onViewDetails(serverId)
MCPComponent->>MCPComponent: Set selectedServerId state
MCPComponent->>MCPDetailView: Render detail view (server info + tools)
User->>MCPDetailView: Review server details
User->>MCPDetailView: Click "Back" button
MCPDetailView->>MCPComponent: Call handleBackToList()
MCPComponent->>MCPComponent: Clear selectedServerId
MCPComponent->>ServerListItem: Render server list again
Note over User,API: Chat Popover Menu Flow
User->>ChatComponent: Click more menu button
ChatComponent->>ChatComponent: Set moreMenuOpen(true)
ChatComponent->>User: Show popover menu
User->>ChatComponent: Click "Export CSV" or "Clear Chat"
ChatComponent->>ChatComponent: Execute action
ChatComponent->>ChatComponent: Set moreMenuOpen(false)
ChatComponent->>User: Close popover menu
Note over User,SettingsModal: SSO Tab Visibility Logic
User->>SettingsModal: Open settings modal
SettingsModal->>SettingsModal: Check environment (hosted/self-hosted)
alt Self-hosted environment
SettingsModal->>SettingsModal: Check isSSOEnabled env var
alt SSO enabled
SettingsModal->>SettingsModal: Check if providers exist
SettingsModal->>User: Show SSO tab (if no providers or is provider owner)
else SSO not enabled
SettingsModal->>User: Hide SSO tab
end
else Hosted environment
SettingsModal->>SettingsModal: Check org + plan + permissions
SettingsModal->>User: Show/hide SSO tab accordingly
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20 files reviewed, 2 comments
| {authType === 'email' ? 'Allowed emails' : 'Allowed SSO emails'} | ||
| </Label> | ||
| <div className='scrollbar-hide flex max-h-32 min-h-9 flex-wrap items-center gap-x-[8px] gap-y-[4px] overflow-y-auto rounded-[4px] border border-[var(--surface-11)] bg-[var(--surface-6)] px-[6px] py-[4px] focus-within:outline-none dark:bg-[var(--surface-9)]'> | ||
| <div className='scrollbar-hide flex max-h-32 flex-wrap items-center gap-x-[8px] gap-y-[4px] overflow-y-auto rounded-[4px] border border-[var(--surface-11)] bg-[var(--surface-6)] px-[8px] py-[6px] focus-within:outline-none dark:bg-[var(--surface-9)]'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: min-h-9 removed - container might collapse when empty
| <div className='scrollbar-hide flex max-h-32 flex-wrap items-center gap-x-[8px] gap-y-[4px] overflow-y-auto rounded-[4px] border border-[var(--surface-11)] bg-[var(--surface-6)] px-[8px] py-[6px] focus-within:outline-none dark:bg-[var(--surface-9)]'> | |
| <div className='scrollbar-hide flex min-h-9 max-h-32 flex-wrap items-center gap-x-[8px] gap-y-[4px] overflow-y-auto rounded-[4px] border border-[var(--surface-11)] bg-[var(--surface-6)] px-[8px] py-[6px] focus-within:outline-none dark:bg-[var(--surface-9)]'> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx
Line: 783:783
Comment:
**logic:** `min-h-9` removed - container might collapse when empty
```suggestion
<div className='scrollbar-hide flex min-h-9 max-h-32 flex-wrap items-center gap-x-[8px] gap-y-[4px] overflow-y-auto rounded-[4px] border border-[var(--surface-11)] bg-[var(--surface-6)] px-[8px] py-[6px] focus-within:outline-none dark:bg-[var(--surface-9)]'>
```
How can I resolve this? If you propose a fix, please make it concise.| 'h-6 min-w-[180px] flex-1 border-none bg-transparent p-0 text-[13px] focus-visible:ring-0 focus-visible:ring-offset-0', | ||
| emails.length > 0 || invalidEmails.length > 0 ? 'pl-[4px]' : 'pl-[4px]' | ||
| )} | ||
| className='min-w-[180px] flex-1 border-none bg-transparent p-0 font-medium font-sans text-foreground text-sm outline-none placeholder:text-[var(--text-muted)] disabled:cursor-not-allowed disabled:opacity-50' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: text-foreground doesn't follow codebase CSS variable pattern - should use text-[var(--text-primary)]
| className='min-w-[180px] flex-1 border-none bg-transparent p-0 font-medium font-sans text-foreground text-sm outline-none placeholder:text-[var(--text-muted)] disabled:cursor-not-allowed disabled:opacity-50' | |
| className='min-w-[180px] flex-1 border-none bg-transparent p-0 font-medium font-sans text-[var(--text-primary)] text-sm outline-none placeholder:text-[var(--text-muted)] disabled:cursor-not-allowed disabled:opacity-50' |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx
Line: 813:813
Comment:
**style:** `text-foreground` doesn't follow codebase CSS variable pattern - should use `text-[var(--text-primary)]`
```suggestion
className='min-w-[180px] flex-1 border-none bg-transparent p-0 font-medium font-sans text-[var(--text-primary)] text-sm outline-none placeholder:text-[var(--text-muted)] disabled:cursor-not-allowed disabled:opacity-50'
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
Fixed settings modal UI and logic, panel, and some modals.
Type of Change
Testing
Solo.
Checklist