Skip to content

Conversation

@emir-karabeg
Copy link
Collaborator

@emir-karabeg emir-karabeg commented Dec 18, 2025

Summary

Light mode, EMCN, many UI/UX improvements.

Type of Change

  • New feature

Testing

Solo.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • 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 Dec 18, 2025

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Dec 26, 2025 8:41pm

@emir-karabeg
Copy link
Collaborator Author

@greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 20, 2025

Greptile Summary

This PR implements comprehensive light mode support, introduces a note block with YouTube embeds, and modernizes the EMCN component library with expanded variants.

Major changes:

  • Light mode implementation: Added warm color palette for light mode with sage gray accents, enabled user theme preference (light/dark/system) in app while forcing light on marketing pages
  • Note block feature: Simplified to markdown-only with automatic YouTube video embedding, removed format dropdown, refined spacing and typography
  • EMCN components: Expanded Badge with 10+ status color variants and dot/icon support, added Button size variants (sm/md/lg) and destructive variant, consolidated theme tokens across Input/Textarea/other components
  • UI/UX improvements: Replaced inline styles with semantic badge variants across workflow blocks, updated progress indicators, refined skeleton loading states, improved Combobox integration

Observations:

  • The PR properly follows the style guide by removing duplicate dark mode classes and using semantic color tokens
  • Badge component expansion provides consistent status visualization across the app
  • Theme system correctly handles the transition between forced and user-selected themes

Confidence Score: 4/5

  • This PR is safe to merge with one known calculation bug that needs fixing
  • The PR successfully implements light mode, note blocks, and EMCN improvements with mostly solid code. However, there's a calculation mismatch in the files progress bar (12 segments rendered vs 16 used in calculation) that should be fixed before merge. The theme system changes are well-implemented, component updates follow project conventions, and the note block feature is cleanly executed.
  • apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/files/files.tsx - Fix progress bar segment calculation mismatch

Important Files Changed

Filename Overview
apps/sim/app/_styles/globals.css Comprehensive color token overhaul for light/dark themes with warm palette for light mode and consistent theme variables
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx Enhanced note block with YouTube embed detection, refined markdown styling, and improved spacing/layout
apps/sim/components/emcn/components/button/button.tsx Added size variants, destructive variant, updated colors for light mode compatibility, simplified hover states
apps/sim/components/emcn/components/badge/badge.tsx Major expansion with 10+ status color variants, size variants (sm/md/lg), dot indicators, and icon support
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/files/files.tsx Updated progress bar styling and layout, but contains calculation mismatch (12 segments rendered, 16 used in calculation)
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/subscription/subscription.tsx Refactored subscription UI with Combobox for workspace admin selection, improved skeleton loading states, reorganized imports

Sequence Diagram

sequenceDiagram
    participant User
    participant ThemeProvider
    participant LocalStorage
    participant NextThemes
    participant Document
    participant Component
    
    User->>ThemeProvider: Opens app
    ThemeProvider->>ThemeProvider: Check pathname
    alt Marketing/Auth pages
        ThemeProvider->>NextThemes: Force light mode
    else App pages
        ThemeProvider->>LocalStorage: Read sim-theme preference
        LocalStorage-->>ThemeProvider: Return theme (system/light/dark)
        ThemeProvider->>NextThemes: Apply user preference
    end
    
    NextThemes->>Document: Apply theme class
    Document->>Document: Load CSS variables
    alt Light mode
        Document->>Document: Apply warm palette tokens
    else Dark mode
        Document->>Document: Apply dark neutral tokens
    end
    
    Document->>Component: Render with theme tokens
    Component->>Component: Use var(--surface-*) variables
    Component->>Component: Use var(--text-*) variables
    Component->>Component: Use var(--border-*) variables
    Component-->>User: Display themed UI
    
    User->>Component: Toggle theme setting
    Component->>LocalStorage: Update sim-theme
    LocalStorage->>NextThemes: Dispatch storage event
    NextThemes->>Document: Update theme class
    Document->>Component: Re-render with new theme
    Component-->>User: Display updated UI
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.

Additional Comments (2)

  1. apps/sim/app/invite/components/status-card.tsx, line 7 (link)

    style: Loader2 import still present but no longer used in the component

    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!

  2. apps/sim/components/ui/tag-input.tsx, line 79 (link)

    style: Redundant conditional logic - both branches set the same pl-[4px] value

    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!

133 files reviewed, 10 comments

Edit Code Review Agent Settings | Greptile


const textareaVariants = cva(
'flex w-full rounded-[4px] border border-[var(--surface-11)] bg-[var(--surface-6)] dark:bg-[var(--surface-9)] px-[8px] py-[8px] font-medium font-sans text-sm text-[var(--text-primary)] transition-colors placeholder:text-[var(--text-muted)] dark:placeholder:text-[var(--text-muted)] outline-none focus:outline-none focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 resize-none overflow-auto disabled:cursor-not-allowed disabled:opacity-50',
'flex w-full rounded-[4px] border border-[var(--border-1)] bg-[var(--surface-5)] dark:bg-[var(--surface-5)] px-[8px] py-[8px] font-medium font-sans text-sm text-[var(--text-primary)] transition-colors placeholder:text-[var(--text-muted)] dark:placeholder:text-[var(--text-muted)] outline-none focus:outline-none focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 resize-none overflow-auto disabled:cursor-not-allowed disabled:opacity-50',
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Redundant dark class - both light and dark use same value --surface-5

Suggested change
'flex w-full rounded-[4px] border border-[var(--border-1)] bg-[var(--surface-5)] dark:bg-[var(--surface-5)] px-[8px] py-[8px] font-medium font-sans text-sm text-[var(--text-primary)] transition-colors placeholder:text-[var(--text-muted)] dark:placeholder:text-[var(--text-muted)] outline-none focus:outline-none focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 resize-none overflow-auto disabled:cursor-not-allowed disabled:opacity-50',
'flex w-full rounded-[4px] border border-[var(--border-1)] bg-[var(--surface-5)] px-[8px] py-[8px] font-medium font-sans text-sm text-[var(--text-primary)] transition-colors placeholder:text-[var(--text-muted)] dark:placeholder:text-[var(--text-muted)] outline-none focus:outline-none focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 resize-none overflow-auto disabled:cursor-not-allowed disabled:opacity-50',

Context Used: Context from dashboard - Tailwind CSS and styling conventions (source)

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!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/components/emcn/components/textarea/textarea.tsx
Line: 6:6

Comment:
**style:** Redundant dark class - both light and dark use same value `--surface-5`

```suggestion
  'flex w-full rounded-[4px] border border-[var(--border-1)] bg-[var(--surface-5)] px-[8px] py-[8px] font-medium font-sans text-sm text-[var(--text-primary)] transition-colors placeholder:text-[var(--text-muted)] dark:placeholder:text-[var(--text-muted)] outline-none focus:outline-none focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 resize-none overflow-auto disabled:cursor-not-allowed disabled:opacity-50',
```

**Context Used:** Context from `dashboard` - Tailwind CSS and styling conventions ([source](https://app.greptile.com/review/custom-context?memory=e1e9ca56-9819-463f-be67-0d2ba849ec4f))

<sub>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!</sub>

How can I resolve this? If you propose a fix, please make it concise.

key: 'sim-theme',
newValue: forcedTheme,
newValue: theme,
oldValue: localStorage.getItem('sim-theme'),
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: retrieving the old value after setting the new value will always return the new value, creating incorrect event data

Suggested change
oldValue: localStorage.getItem('sim-theme'),
const oldValue = localStorage.getItem('sim-theme')
localStorage.setItem('sim-theme', theme)
window.dispatchEvent(
new StorageEvent('storage', {
key: 'sim-theme',
newValue: theme,
oldValue: oldValue,
storageArea: localStorage,
url: window.location.href,
})
)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/core/utils/theme.ts
Line: 19:19

Comment:
**logic:** retrieving the old value after setting the new value will always return the new value, creating incorrect event data

```suggestion
      const oldValue = localStorage.getItem('sim-theme')
  localStorage.setItem('sim-theme', theme)

  window.dispatchEvent(
    new StorageEvent('storage', {
      key: 'sim-theme',
      newValue: theme,
      oldValue: oldValue,
      storageArea: localStorage,
      url: window.location.href,
    })
  )
```

How can I resolve this? If you propose a fix, please make it concise.

secondary: 'bg-[var(--brand-secondary)] text-[var(--text-primary)]',
tertiary:
'bg-[var(--brand-tertiary)] dark:bg-[var(--brand-tertiary)] dark:text-[var(--text-primary)] text-[var(--text-primary)] hover:bg-[var(--brand-tertiary)] hover:dark:bg-[var(--brand-tertiary)] hover:text-[var(--text-primary)] hover:dark:text-[var(--text-primary)]',
'!bg-[var(--brand-tertiary-2)] !text-[var(--text-inverse)] hover:brightness-110 hover:!text-[var(--text-inverse)] ![transition-property:background-color,border-color,fill,stroke]',
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Using !important overrides suggests this variant has styling conflicts. Consider if the base styles can be adjusted to avoid needing !important. Is there a specific reason this variant needs to override the transition property with !important?

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!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/components/emcn/components/button/button.tsx
Line: 21:21

Comment:
**style:** Using !important overrides suggests this variant has styling conflicts. Consider if the base styles can be adjusted to avoid needing !important. Is there a specific reason this variant needs to override the transition property with !important?

<sub>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!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 490 to 491
{Array.from({ length: 12 }).map((_, i) => {
const filledCount = Math.ceil((Math.min(storageInfo.percentUsed, 100) / 100) * 16)
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Progress bar calculation mismatch: using 12 segments in the loop but calculating filledCount based on 16 segments

Suggested change
{Array.from({ length: 12 }).map((_, i) => {
const filledCount = Math.ceil((Math.min(storageInfo.percentUsed, 100) / 100) * 16)
{Array.from({ length: 12 }).map((_, i) => {
const filledCount = Math.ceil((Math.min(storageInfo.percentUsed, 100) / 100) * 12)

Should the calculation use 12 segments to match the loop, or should the loop create 16 segments to match the calculation?

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/files/files.tsx
Line: 490:491

Comment:
**logic:** Progress bar calculation mismatch: using 12 segments in the loop but calculating filledCount based on 16 segments

```suggestion
            {Array.from({ length: 12 }).map((_, i) => {
              const filledCount = Math.ceil((Math.min(storageInfo.percentUsed, 100) / 100) * 12)
```

 Should the calculation use 12 segments to match the loop, or should the loop create 16 segments to match the calculation?

How can I resolve this? If you propose a fix, please make it concise.

onMouseEnter={() => setIsHoveringMessage(true)}
onMouseLeave={() => setIsHoveringMessage(false)}
className='group relative w-full cursor-pointer rounded-[4px] border border-[var(--surface-11)] bg-[var(--surface-6)] px-[6px] py-[6px] transition-all duration-200 hover:border-[var(--surface-14)] hover:bg-[var(--surface-9)] dark:bg-[var(--surface-9)] dark:hover:border-[var(--surface-13)] dark:hover:bg-[var(--surface-11)]'
className='group relative w-full cursor-pointer rounded-[4px] border border-[var(--border-1)] bg-[var(--surface-5)] px-[6px] py-[6px] transition-all duration-200 hover:border-[var(--surface-14)] hover:bg-[var(--surface-5)] dark:bg-[var(--surface-5)] dark:hover:border-[var(--surface-13)] dark:hover:bg-[var(--border-1)]'
Copy link
Contributor

Choose a reason for hiding this comment

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

style: The dark mode hover state uses dark:hover:bg-[var(--border-1)] which seems inconsistent - using a border color for background might create unexpected visual results. Is using --border-1 as a background color in dark mode hover state intentional, or should this be a surface color?

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!

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/copilot/components/copilot-message/copilot-message.tsx
Line: 314:314

Comment:
**style:** The dark mode hover state uses `dark:hover:bg-[var(--border-1)]` which seems inconsistent - using a border color for background might create unexpected visual results. Is using `--border-1` as a background color in dark mode hover state intentional, or should this be a surface color?

<sub>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!</sub>

How can I resolve this? If you propose a fix, please make it concise.

{/* Gradient fade when truncated - applies to entire message box */}
{!isExpanded && needsExpansion && (
<div className='pointer-events-none absolute right-0 bottom-0 left-0 h-6 bg-gradient-to-t from-0% from-[var(--surface-6)] via-40% via-[var(--surface-6)]/70 to-100% to-transparent group-hover:from-[var(--surface-9)] group-hover:via-[var(--surface-9)]/70 dark:from-[var(--surface-9)] dark:via-[var(--surface-9)]/70 dark:group-hover:from-[var(--surface-11)] dark:group-hover:via-[var(--surface-11)]/70' />
<div className='pointer-events-none absolute right-0 bottom-0 left-0 h-6 bg-gradient-to-t from-0% from-[var(--surface-5)] via-40% via-[var(--surface-5)]/70 to-100% to-transparent group-hover:from-[var(--surface-5)] group-hover:via-[var(--surface-5)]/70 dark:from-[var(--surface-5)] dark:via-[var(--surface-5)]/70 dark:group-hover:from-[var(--border-1)] dark:group-hover:via-[var(--border-1)]/70' />
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Similar inconsistency in gradient overlay where dark mode hover uses --border-1 for gradient colors instead of surface colors

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!

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/copilot/components/copilot-message/copilot-message.tsx
Line: 360:360

Comment:
**style:** Similar inconsistency in gradient overlay where dark mode hover uses `--border-1` for gradient colors instead of surface colors

<sub>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!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +428 to +430
<span className='truncate font-base text-[14px] text-[var(--text-primary)] dark:text-[var(--white)]'>
{activeWorkspace?.name || 'Loading...'}
</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

style: inconsistent dark mode text styling - active popover button uses text-[var(--text-primary)] (line 284) while disabled fallback uses dark:text-[var(--white)]

Suggested change
<span className='truncate font-base text-[14px] text-[var(--text-primary)] dark:text-[var(--white)]'>
{activeWorkspace?.name || 'Loading...'}
</span>
<span className='truncate font-base text-[14px] text-[var(--text-primary)]'>
{activeWorkspace?.name || 'Loading...'}
</span>

Context Used: Context from dashboard - Tailwind CSS and styling conventions (source)

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!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx
Line: 428:430

Comment:
**style:** inconsistent dark mode text styling - active popover button uses `text-[var(--text-primary)]` (line 284) while disabled fallback uses `dark:text-[var(--white)]`

```suggestion
            <span className='truncate font-base text-[14px] text-[var(--text-primary)]'>
              {activeWorkspace?.name || 'Loading...'}
            </span>
```

**Context Used:** Context from `dashboard` - Tailwind CSS and styling conventions ([source](https://app.greptile.com/review/custom-context?memory=e1e9ca56-9819-463f-be67-0d2ba849ec4f))

<sub>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!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 451 to 476
<Button
className='h-[28px] truncate px-[8px] py-[5px] text-[12.5px] hover:bg-[var(--surface-9)] hover:text-[var(--text-primary)]'
className={`h-[28px] truncate rounded-[6px] border px-[8px] py-[5px] text-[12.5px] ${
_hasHydrated && activeTab === 'copilot'
? 'border-[var(--border-1)]'
: 'border-transparent hover:border-[var(--border-1)] hover:bg-[var(--surface-5)] hover:text-[var(--text-primary)]'
}`}
variant={_hasHydrated && activeTab === 'copilot' ? 'active' : 'ghost'}
onClick={() => handleTabClick('copilot')}
data-tab-button='copilot'
>
Copilot
</Button>
<Button
className='h-[28px] px-[8px] py-[5px] text-[12.5px] hover:bg-[var(--surface-9)] hover:text-[var(--text-primary)]'
className={`h-[28px] rounded-[6px] border px-[8px] py-[5px] text-[12.5px] ${
_hasHydrated && activeTab === 'toolbar'
? 'border-[var(--border-1)]'
: 'border-transparent hover:border-[var(--border-1)] hover:bg-[var(--surface-5)] hover:text-[var(--text-primary)]'
}`}
variant={_hasHydrated && activeTab === 'toolbar' ? 'active' : 'ghost'}
onClick={() => handleTabClick('toolbar')}
data-tab-button='toolbar'
>
Toolbar
</Button>
<Button
className='h-[28px] px-[8px] py-[5px] text-[12.5px] hover:bg-[var(--surface-9)] hover:text-[var(--text-primary)]'
className={`h-[28px] rounded-[6px] border px-[8px] py-[5px] text-[12.5px] ${
_hasHydrated && activeTab === 'editor'
? 'border-[var(--border-1)]'
: 'border-transparent hover:border-[var(--border-1)] hover:bg-[var(--surface-5)] hover:text-[var(--text-primary)]'
}`}
variant={_hasHydrated && activeTab === 'editor' ? 'active' : 'ghost'}
onClick={() => handleTabClick('editor')}
data-tab-button='editor'
>
Editor
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

style: The custom conditional styling with template literals duplicates similar logic across three tab buttons, creating maintenance overhead.

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!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx
Line: 451:486

Comment:
**style:** The custom conditional styling with template literals duplicates similar logic across three tab buttons, creating maintenance overhead.

<sub>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!</sub>

How can I resolve this? If you propose a fix, please make it concise.

@emir-karabeg
Copy link
Collaborator Author

@greptile

@emir-karabeg
Copy link
Collaborator Author

@greptile

@emir-karabeg emir-karabeg changed the title feat: light, note block, ui/ux feat: light mode Dec 26, 2025
@emir-karabeg emir-karabeg merged commit 1f0e3f2 into staging Dec 26, 2025
10 checks passed
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