Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
e057c34 to
7deeb50
Compare
|
@greptile |
Greptile SummaryThis PR implements comprehensive light mode support, introduces a note block with YouTube embeds, and modernizes the EMCN component library with expanded variants. Major changes:
Observations:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
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
|
There was a problem hiding this comment.
Additional Comments (2)
-
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!
-
apps/sim/components/ui/tag-input.tsx, line 79 (link)style: Redundant conditional logic - both branches set the same
pl-[4px]valueNote: 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
|
|
||
| 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', |
There was a problem hiding this comment.
style: Redundant dark class - both light and dark use same value --surface-5
| '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.
apps/sim/lib/core/utils/theme.ts
Outdated
| key: 'sim-theme', | ||
| newValue: forcedTheme, | ||
| newValue: theme, | ||
| oldValue: localStorage.getItem('sim-theme'), |
There was a problem hiding this comment.
logic: retrieving the old value after setting the new value will always return the new value, creating incorrect event data
| 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]', |
There was a problem hiding this 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?
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.| {Array.from({ length: 12 }).map((_, i) => { | ||
| const filledCount = Math.ceil((Math.min(storageInfo.percentUsed, 100) / 100) * 16) |
There was a problem hiding this comment.
logic: Progress bar calculation mismatch: using 12 segments in the loop but calculating filledCount based on 16 segments
| {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)]' |
There was a problem hiding this 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?
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' /> |
There was a problem hiding this comment.
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.| <span className='truncate font-base text-[14px] text-[var(--text-primary)] dark:text-[var(--white)]'> | ||
| {activeWorkspace?.name || 'Loading...'} | ||
| </span> |
There was a problem hiding this 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)]
| <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.| @@ -474,7 +486,7 @@ export function Panel() { | |||
| </Button> | |||
There was a problem hiding this comment.
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.359fec3 to
db74927
Compare
db74927 to
93c9cfc
Compare
|
@greptile |
d037cf7 to
8ba241d
Compare
8ba241d to
deee7d6
Compare
|
@greptile |
38a7ed9 to
71696ab
Compare
71696ab to
b694837
Compare
Summary
Light mode, EMCN, many UI/UX improvements.
Type of Change
Testing
Solo.
Checklist