Dark mode polish #19370
Dark mode polish #19370
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
dannyroosevelt
moved this to Ready for PR Review
in Component (Source and Action) Backlog
WalkthroughAdds a package entry point and introduces theme-aware react-select styling: new select-styles utilities, new theme color tokens, updates to Select components to consume customization context and merge external styles/components, and two new customization component keys in types. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
packages/connect-react/package.json(1 hunks)packages/connect-react/src/components/ControlApp.tsx(3 hunks)packages/connect-react/src/components/SelectApp.tsx(7 hunks)packages/connect-react/src/components/SelectComponent.tsx(5 hunks)packages/connect-react/src/hooks/customization-context.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
📚 Learning: 2025-08-27T16:47:38.177Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Applied to files:
packages/connect-react/src/components/ControlApp.tsxpackages/connect-react/src/components/SelectComponent.tsxpackages/connect-react/src/hooks/customization-context.tsxpackages/connect-react/package.jsonpackages/connect-react/src/components/SelectApp.tsx
📚 Learning: 2025-08-27T16:48:48.776Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
Applied to files:
packages/connect-react/src/hooks/customization-context.tsxpackages/connect-react/package.json
🧬 Code graph analysis (2)
packages/connect-react/src/components/SelectComponent.tsx (1)
packages/connect-react/src/hooks/customization-context.tsx (2)
useCustomize(259-330)BaseReactSelectProps(133-140)
packages/connect-react/src/components/SelectApp.tsx (1)
packages/connect-react/src/hooks/customization-context.tsx (2)
useCustomize(259-330)BaseReactSelectProps(133-140)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (4)
packages/connect-react/src/hooks/customization-context.tsx (1)
57-58: Clarify the intent of usingnevertype for these properties.Using
neverforselectAppandselectComponentmeans these keys can exist in the type but cannot be assigned any value. If the intent is to allow customization viaselect.getProps("selectApp", ...)andselect.getProps("selectComponent", ...)as seen in other files, consider using a properReactSelectComponentsConfigtype instead, similar tocontrolAppSelectandcontrolSelect.If
neveris intentional to prevent direct assignment while still allowing the key to exist for lookup purposes, a brief comment explaining this would help future maintainers.packages/connect-react/package.json (1)
9-9: LGTM!Adding the
mainfield improves compatibility with older bundlers and tools that don't fully support theexportsfield, while staying consistent with the existingbrowserandexportsconfiguration.packages/connect-react/src/components/SelectComponent.tsx (1)
170-173: Component merge order may prevent user customization ofMenuList.The current order spreads
selectProps.componentsfirst, thencustomComponents. This means the internalMenuList(with loading indicator) andIndicatorSeparatorwill always override any user-provided components through the customization context.If this is intentional (to preserve the loading indicator behavior), consider documenting this limitation. Otherwise, consider conditionally merging or allowing users to wrap the internal components.
packages/connect-react/src/components/SelectApp.tsx (1)
238-241: Component merge order may prevent user customization.Same issue as
SelectComponent.tsx: the internalcustomComponents(Option, SingleValue, MenuList, IndicatorSeparator) will always override any user-provided components from the customization context due to the spread order.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/connect-react/src/components/SelectApp.tsx (1)
217-224: Hardcoded loading indicator color.The loading indicator text uses
color: "#666"which doesn't follow the theme-aware approach. Consider using the resolvedtextcolor for consistency:<div style={{ padding: "8px 12px", textAlign: "center", - color: "#666", + color: text, fontSize: "14px", }}>Note: This would require passing
textinto the memoized component or restructuring to access the color.
♻️ Duplicate comments (2)
packages/connect-react/src/components/SelectComponent.tsx (1)
64-69: Hardcoded hover/selected state colors.These background colors for hover and selected states are hardcoded and not derived from the theme. This was flagged in a previous review.
packages/connect-react/src/components/SelectApp.tsx (1)
98-118: DuplicateresolveColorhelper and hardcoded state colors.This helper and the hardcoded hover/selected colors are duplicated from
SelectComponent.tsx. This was already flagged in a previous review suggesting extraction to a shared utility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/connect-react/src/components/SelectApp.tsx(7 hunks)packages/connect-react/src/components/SelectComponent.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
📚 Learning: 2025-08-27T16:47:38.177Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Applied to files:
packages/connect-react/src/components/SelectApp.tsxpackages/connect-react/src/components/SelectComponent.tsx
🧬 Code graph analysis (2)
packages/connect-react/src/components/SelectApp.tsx (2)
packages/connect-react/src/hooks/customization-context.tsx (2)
useCustomize(259-330)BaseReactSelectProps(133-140)types/src/index.ts (1)
App(170-178)
packages/connect-react/src/components/SelectComponent.tsx (1)
packages/connect-react/src/hooks/customization-context.tsx (2)
useCustomize(259-330)BaseReactSelectProps(133-140)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
packages/connect-react/src/components/SelectComponent.tsx (4)
14-18: LGTM on imports.The imports for
defaultTheme,useCustomize, andBaseReactSelectPropsare correctly aligned with the customization pattern used in this codebase.
92-130: LGTM on theme-aware select styling.The
baseSelectPropscorrectly applies theme-derived colors to control, menu, singleValue, input, and option elements. The option state handling for selected/focused combinations is well-structured.
172-175: LGTM on component merging order.The merge order correctly prioritizes
customComponentsoverselectProps.components, ensuring local overrides (likeMenuListandIndicatorSeparator) take precedence while still allowing external customization.
182-188: LGTM on styles merging.The nullish coalescing and spread order correctly handle the case where
selectProps.stylesmay be undefined while ensuring themenuPortalz-index override is always applied.packages/connect-react/src/components/SelectApp.tsx (3)
9-13: LGTM on imports.The imports align with the customization pattern and are consistent with
SelectComponent.tsx.
238-243: LGTM on props and styles merging.The spread order and merging pattern correctly prioritizes local customizations while preserving external customization options from
selectProps.Also applies to: 263-269
77-85: LGTM on selectedValue memoization.The memoization correctly depends on
value?.nameSlugrather than the fullvalueobject, preventing unnecessary recalculations when only other properties change.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/connect-react/src/components/SelectApp.tsx (1)
189-198: Hardcoded loading indicator color doesn't follow theming pattern.The loading indicator uses hardcoded
color: "#666"which doesn't adapt to the theme. For consistency with the theming approach, consider deriving this from a theme token.{isLoadingMoreRef.current && ( <div style={{ padding: "8px 12px", textAlign: "center", - color: "#666", + color: text, // or create a new token like textMuted fontSize: "14px", }}> Loading more apps... </div> )}This same pattern applies to
SelectComponent.tsxlines 114-123.
♻️ Duplicate comments (3)
packages/connect-react/src/components/ControlApp.tsx (1)
55-72: ExtractresolveColorto a shared utility to reduce duplication.This helper is duplicated across
ControlApp.tsx,SelectApp.tsx, andSelectComponent.tsx. Since you've already createdselect-styles.tsfor shared styling utilities, consider addingresolveColorthere as well.// In packages/connect-react/src/utils/select-styles.ts or a new theme-utils.ts: +import type { Colors } from "../theme"; + +export const resolveColor = ( + themeColors: Partial<Colors>, + key: keyof Colors, + fallback: string, +): string => { + const current = themeColors[key]; + return current !== undefined ? current : fallback; +};Then import and use it in all three components to maintain DRY principles.
packages/connect-react/src/components/SelectComponent.tsx (1)
50-67: DuplicateresolveColorhelper - same as other components.This is the same
resolveColorhelper duplicated fromControlApp.tsxandSelectApp.tsx. Please consolidate this into the sharedselect-styles.tsutility.packages/connect-react/src/components/SelectApp.tsx (1)
98-116: DuplicateresolveColorhelper - third instance.This is the third copy of
resolveColoracross the three select components. Please consolidate intoselect-styles.ts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
packages/connect-react/src/components/ControlApp.tsx(3 hunks)packages/connect-react/src/components/SelectApp.tsx(7 hunks)packages/connect-react/src/components/SelectComponent.tsx(5 hunks)packages/connect-react/src/hooks/customization-context.tsx(1 hunks)packages/connect-react/src/theme.ts(1 hunks)packages/connect-react/src/utils/select-styles.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
📚 Learning: 2025-08-27T16:47:38.177Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Applied to files:
packages/connect-react/src/components/SelectApp.tsxpackages/connect-react/src/hooks/customization-context.tsxpackages/connect-react/src/components/SelectComponent.tsxpackages/connect-react/src/components/ControlApp.tsxpackages/connect-react/src/utils/select-styles.ts
📚 Learning: 2025-08-27T16:48:48.776Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
Applied to files:
packages/connect-react/src/hooks/customization-context.tsxpackages/connect-react/src/components/ControlApp.tsx
📚 Learning: 2025-08-27T17:25:10.425Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/utils/type-guards.ts:23-33
Timestamp: 2025-08-27T17:25:10.425Z
Learning: In the connect-react package, the isOptionWithLabel type guard intentionally restricts value types to string|number for runtime filtering purposes, even though LabelValueOption<T> allows any T. This runtime behavior should be preserved over type safety improvements.
Applied to files:
packages/connect-react/src/components/ControlApp.tsx
🧬 Code graph analysis (3)
packages/connect-react/src/components/SelectComponent.tsx (2)
packages/connect-react/src/hooks/customization-context.tsx (2)
useCustomize(261-332)BaseReactSelectProps(135-142)packages/connect-react/src/utils/select-styles.ts (1)
createBaseSelectStyles(25-83)
packages/connect-react/src/components/ControlApp.tsx (2)
packages/connect-react/src/utils/select-styles.ts (1)
createBaseSelectStyles(25-83)packages/connect-react/src/hooks/customization-context.tsx (1)
BaseReactSelectProps(135-142)
packages/connect-react/src/utils/select-styles.ts (1)
packages/connect-react/src/theme.ts (1)
Shadows(75-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (6)
packages/connect-react/src/components/ControlApp.tsx (1)
88-112: LGTM!The integration with
createBaseSelectStylesis well-structured. The style merging pattern correctly composes the base styles with the control-specificgridAreaproperty.packages/connect-react/src/utils/select-styles.ts (1)
1-83: Well-designed shared utility that addresses previous duplication concerns.This new file effectively centralizes the react-select styling logic that was previously duplicated across components. The type definitions are clear, and the option state handling correctly covers all combinations (
isSelected && isFocused,isSelected,isFocused).packages/connect-react/src/components/SelectComponent.tsx (2)
91-106: LGTM!The
baseSelectPropsconstruction and integration withselect.getProps("selectComponent", baseSelectProps)follows the established pattern correctly.
146-162: LGTM! Correct component and style composition order.The spreading order correctly prioritizes local customizations (
customComponents) over external ones (selectProps.components), and properly merges the portal z-index style with base styles.packages/connect-react/src/components/SelectApp.tsx (2)
136-207: LGTM!appIconBgcorrectly integrated into custom components.The app icon background color is now theme-aware via the
appIconBgtoken derived fromresolveColor("appIconBackground", "#fff"). The dependency array correctly includesappIconBgto trigger re-memoization when the theme changes.
208-244: LGTM! Select component props correctly integrated.The
selectPropsspread and subsequent component/style merging follows the correct pattern, ensuring external customizations are preserved while local overrides take precedence.
| export type ReactSelectComponents = { | ||
| controlAppSelect: typeof ControlApp; | ||
| controlSelect: typeof ControlSelect; | ||
| // SelectApp and SelectComponent use react-select but don't have specific component types | ||
| // They just use the customization system for styles/classNames | ||
| selectApp: typeof ControlApp; | ||
| selectComponent: typeof ControlSelect; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type extension looks good, but naming may cause confusion.
The ReactSelectComponents type now has both controlAppSelect/controlSelect and selectApp/selectComponent. The comment helps clarify the distinction, but the naming overlap between controlAppSelect vs selectApp could be confusing for consumers.
Consider adding a brief JSDoc or expanding the inline comment to clarify when to use each key (e.g., controlAppSelect for ControlApp component customization vs selectApp for SelectApp component customization).
🧰 Tools
🪛 Biome (2.1.2)
[error] 54-54: Shouldn't redeclare 'ReactSelectComponents'. Consider to delete it or rename it.
'ReactSelectComponents' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In packages/connect-react/src/hooks/customization-context.tsx around lines 54 to
61, the ReactSelectComponents type has overlapping names
(controlAppSelect/controlSelect vs selectApp/selectComponent) that can confuse
consumers; add concise JSDoc comments above each property (controlAppSelect,
controlSelect, selectApp, selectComponent) explaining exactly which component
each key customizes (e.g., "controlAppSelect — overrides ControlApp's
react-select control", "selectApp — styling/className hooks for SelectApp
only"), or expand the existing inline comment to state the same, so callers know
when to use each key.
|
|
||
| // Option state backgrounds (dark mode friendly) | ||
| // select.option:hover:backgroundColor | ||
| optionHover?: string; | ||
| // select.option:selected:backgroundColor | ||
| optionSelected?: string; | ||
| // select.option:selected:hover:backgroundColor | ||
| optionSelectedHover?: string; | ||
|
|
||
| // App icon background color (for visibility in dark mode) | ||
| appIconBackground?: string; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! New color tokens are well-documented.
The new optional color tokens (optionHover, optionSelected, optionSelectedHover, appIconBackground) extend the theming system appropriately for dark mode support. The inline comments clearly document their purpose.
Consider adding default values for these tokens in defaultTheme (lines 106-128) to provide light-mode defaults, ensuring components don't need to rely solely on hardcoded fallbacks.
export const defaultTheme: Theme = {
// ...existing colors...
colors: {
// ...existing...
neutral90: "hsl(0, 0%, 10%)",
+
+ // Option state backgrounds (light mode defaults)
+ optionHover: "hsl(0, 0%, 95%)",
+ optionSelected: "rgba(38, 132, 255, 0.1)",
+ optionSelectedHover: "rgba(38, 132, 255, 0.2)",
+ appIconBackground: "#fff",
},
// ...
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/connect-react/src/theme.ts around lines 62-73 (and add defaults in
defaultTheme at lines ~106-128), the new optional color tokens optionHover,
optionSelected, optionSelectedHover, and appIconBackground need light-mode
default values so components don't rely on hardcoded fallbacks; add these four
keys to the defaultTheme object with sensible CSS color strings (matching the
existing token naming and value format used in that file), ensure types remain
compatible (no breaking changes), and run a quick build/test to confirm no type
or import errors.
| option: ( | ||
| base: CSSObjectWithLabel, | ||
| state: { isSelected: boolean; isFocused: boolean }, | ||
| ) => { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using react-select's full option state type.
The inline type { isSelected: boolean; isFocused: boolean } works but is a subset of react-select's actual OptionState. For better type fidelity and future-proofing, consider importing and using the actual type.
import type {
- CSSObjectWithLabel, GroupBase, StylesConfig,
+ CSSObjectWithLabel, GroupBase, StylesConfig, OptionProps,
} from "react-select";
// ...
option: (
base: CSSObjectWithLabel,
- state: { isSelected: boolean; isFocused: boolean },
+ state: Pick<OptionProps<Option, IsMulti, Group>, 'isSelected' | 'isFocused'>,
) => {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/connect-react/src/utils/select-styles.ts around lines 64 to 67, the
option style function types its state as a minimal inline shape; replace that
with react-select's actual OptionState type by importing it from 'react-select'
and using it for the state parameter (or the appropriate generic
OptionState<TOption, IsMulti> if your project uses generics) so the function
signature uses the official react-select option state type for full type
fidelity and future-proofing.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/connect-react/src/utils/select-styles.ts (1)
90-93: Past review comment still applicable.The inline type
{ isSelected: boolean; isFocused: boolean }is a subset of react-select'sOptionState. The previous review suggested using the full type for better type fidelity and future-proofing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/connect-react/src/utils/select-styles.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-27T16:47:38.177Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Applied to files:
packages/connect-react/src/utils/select-styles.ts
📚 Learning: 2025-08-27T17:25:10.425Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/utils/type-guards.ts:23-33
Timestamp: 2025-08-27T17:25:10.425Z
Learning: In the connect-react package, the isOptionWithLabel type guard intentionally restricts value types to string|number for runtime filtering purposes, even though LabelValueOption<T> allows any T. This runtime behavior should be preserved over type safety improvements.
Applied to files:
packages/connect-react/src/utils/select-styles.ts
📚 Learning: 2025-08-27T16:48:48.776Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
Applied to files:
packages/connect-react/src/utils/select-styles.ts
🧬 Code graph analysis (1)
packages/connect-react/src/utils/select-styles.ts (1)
packages/connect-react/src/theme.ts (2)
Shadows(75-80)Colors(8-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
packages/connect-react/src/utils/select-styles.ts (3)
1-21: Clean type definitions for select theming.The imports and type definitions are well-structured and provide a clear contract for theme-aware select styling.
51-89: Well-structured style configuration.The function signature, type parameters, and style functions for
control,menu,singleValue, andinputare correctly implemented with proper base style spreading and theme application.
94-107: Correct option state handling.The cascading logic for option background colors correctly prioritizes state combinations (selected+focused > selected > focused > default), ensuring proper visual feedback for all interaction states.
| export function resolveSelectColors(colors: Partial<Colors>): SelectColorConfig & { appIconBg: string } { | ||
| const resolve = <K extends keyof Colors>(key: K, fallback: string): string => { | ||
| const current = colors[key]; | ||
| return current !== undefined | ||
| ? current | ||
| : fallback; | ||
| }; | ||
|
|
||
| return { | ||
| surface: resolve("neutral0", "#18181b"), | ||
| border: resolve("neutral20", "rgba(255,255,255,0.16)"), | ||
| text: resolve("neutral80", "#a1a1aa"), | ||
| textStrong: resolve("neutral90", "#e4e4e7"), | ||
| hoverBg: resolve("optionHover", "#27272a"), | ||
| selectedBg: resolve("optionSelected", "rgba(59,130,246,0.2)"), | ||
| selectedHoverBg: resolve("optionSelectedHover", "rgba(59,130,246,0.35)"), | ||
| appIconBg: resolve("appIconBackground", "#fff"), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find theme-related files and configurations
find . -type f \( -name "*theme*" -o -name "*color*" \) \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" \) | head -20Repository: PipedreamHQ/pipedream
Length of output: 102
🏁 Script executed:
# Search for optionHover, optionSelected, appIconBackground usage
rg -n 'optionHover|optionSelected|appIconBackground|optionSelectedHover' --type=ts --type=tsx -B2 -A2Repository: PipedreamHQ/pipedream
Length of output: 92
🏁 Script executed:
# Find SelectColorConfig type definition
rg -n 'SelectColorConfig|Colors' --type=ts --type=tsx | grep -E '(type|interface)' | head -20Repository: PipedreamHQ/pipedream
Length of output: 92
🏁 Script executed:
# Search for light theme definitions
rg -n 'light.*theme|theme.*light' --type=ts --type=tsx -i -B3 -A3Repository: PipedreamHQ/pipedream
Length of output: 92
🏁 Script executed:
# Read the theme.ts file to understand Colors interface
cat -n ./packages/connect-react/src/theme.ts | head -100Repository: PipedreamHQ/pipedream
Length of output: 3627
🏁 Script executed:
# Search for optionHover, optionSelected, appIconBackground without tsx
rg -n 'optionHover|optionSelected|appIconBackground|optionSelectedHover' --type=ts -B2 -A2Repository: PipedreamHQ/pipedream
Length of output: 1669
🏁 Script executed:
# Search for light theme or theme definitions
rg -n 'light|Light|LIGHT' --type=ts ./packages/connect-react/src/ -B1 -A1 | head -40Repository: PipedreamHQ/pipedream
Length of output: 443
🏁 Script executed:
# Find where resolveSelectColors is used
rg -n 'resolveSelectColors' --type=ts -B2 -A2Repository: PipedreamHQ/pipedream
Length of output: 3060
🏁 Script executed:
# Read more of theme.ts to see theme definitions
cat -n ./packages/connect-react/src/theme.ts | tail -100Repository: PipedreamHQ/pipedream
Length of output: 3182
🏁 Script executed:
# Search for all theme definitions (especially light themes)
rg -n 'Theme|theme.*=' --type=ts ./packages/connect-react/src/theme.ts -A10 | head -60Repository: PipedreamHQ/pipedream
Length of output: 1705
🏁 Script executed:
# Check if there's a light theme export
rg -n 'export.*theme|lightTheme|darkTheme|defaultTheme' --type=ts ./packages/connect-react/src/ -B2 -A5Repository: PipedreamHQ/pipedream
Length of output: 6819
🏁 Script executed:
# Look at the default theme values more completely
cat -n ./packages/connect-react/src/theme.ts | sed -n '100,200p'Repository: PipedreamHQ/pipedream
Length of output: 3237
🏁 Script executed:
# Check how resolveSelectColors values are used in components
cat -n ./packages/connect-react/src/components/SelectApp.tsx | sed -n '100,150p'Repository: PipedreamHQ/pipedream
Length of output: 1652
🏁 Script executed:
# Look at how hoverBg and border are applied
rg -n 'hoverBg|selectedBg|selectedHoverBg' --type=ts -A2 -B2 ./packages/connect-react/src/Repository: PipedreamHQ/pipedream
Length of output: 5828
Default light theme has poor contrast for dropdown hover/selected states due to dark mode fallbacks.
The defaultTheme is a light theme (neutral0 = white background) but does not provide the optional color properties (optionHover, optionSelected, optionSelectedHover, appIconBackground). When these are undefined, resolveSelectColors falls back to dark mode colors:
hoverBgdefaults to"#27272a"(dark gray) – nearly invisible on the white surfaceselectedBgdefaults to"rgba(59,130,246,0.2)"(blue) – acceptable contrastselectedHoverBgdefaults to"rgba(59,130,246,0.35)"– acceptable contrastborderdefaults to"rgba(255,255,255,0.16)"(translucent white) – invisible on light backgrounds
Users who don't customize these optional colors will experience poor hover state visibility in the default light theme.
Provide theme-aware fallbacks or require all select option colors to be explicitly defined when customizing the theme.
michelle0927
moved this from Ready for PR Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to Ready for Release
in Component (Source and Action) Backlog
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
WHY
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.