Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions packages/design-system/src/css/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
--code-text-color: rgba(0, 122, 255, 1);
--code-bg-color: rgba(96, 165, 250, 0.2);
--code-block-bg-color: rgba(60, 60, 60, 0.12);

/* Scrollbar tokens (colors reuse existing palette) */
--scrollbar-size: 10px; /* width/height for webkit scrollbars */
--scrollbar-track: var(--color-white);
Copy link

Choose a reason for hiding this comment

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

[quality] high Priority

Issue: Undefined CSS color token dependency
Context: The scrollbar implementation uses --color-white token which is not defined anywhere in the CSS. This will cause the scrollbar to fall back to browser defaults.
Suggestion: Use --color-pure-white which is defined at line 116, or define --color-white token in the color palette section.

--scrollbar-thumb: var(--color-gray-400);
--scrollbar-thumb-hover: var(--color-gray-400);
Copy link

Choose a reason for hiding this comment

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

[quality] medium Priority

Issue: Inconsistent hover behavior for scrollbar thumb
Context: The --scrollbar-thumb-hover is set to the same value as --scrollbar-thumb in light theme, providing no visual feedback on hover.
Suggestion: Set --scrollbar-thumb-hover to a slightly darker shade like var(--color-gray-500) to provide clear hover feedback.

}

@media (prefers-color-scheme: dark) {
Expand All @@ -46,9 +52,27 @@
--content-fg: #fff;
--content-hover-bg: #222;
--content-hover-fg: #fff;
--scrollbar-track: var(--color-charcoal-600);
/* Dark overrides for scrollbar */
--scrollbar-thumb: var(--color-charcoal-100);
--scrollbar-thumb-hover: var(--color-gray-800);
}
}

/* Dark theme variable overrides for class-based theming */
.dark-theme {
/* ensure tokens match dark scheme even without prefers-color-scheme */
--scrollbar-track: var(--color-charcoal-600);
--scrollbar-thumb: var(--color-charcoal-100);
--scrollbar-thumb-hover: var(--color-gray-800);
/* let UA widgets (including scrollbars) render in dark mode where supported */
color-scheme: dark;
}
/* Ensure scrollable containers participate in dark color-scheme */
.dark-theme .custom-scrollbar {
color-scheme: dark;
}

@theme {
--text-xxs: 0.625rem;
--text-xxs--line-height: calc(1 / 0.625);
Expand Down Expand Up @@ -152,6 +176,56 @@
}
}

/* ===================== Custom Scrollbar (cross-browser) =====================
Usage: Add `custom-scrollbar` class to any scrollable container.
Notes:
- Firefox uses `scrollbar-width` and `scrollbar-color`.
- WebKit/Blink (Chrome/Edge/Safari) use `::-webkit-scrollbar` pseudo elements.
- macOS/iOS show overlay scrollbars; thick tracks may appear slimmer there.
- Uses existing CSS variables (tokens) for colors and size.
============================================================================ */
@layer components {
Copy link

Choose a reason for hiding this comment

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

[architecture] medium Priority

Issue: Custom CSS implementation instead of leveraging existing Tailwind utilities
Context: The project already uses a @Utility scrollbar-hide pattern at line 169, but the new scrollbar styling doesn't follow the same utility-first approach.
Suggestion: Consider implementing the custom scrollbar as a @Utility directive to maintain consistency with the existing scrollbar-hide utility and follow Tailwind's utility-first philosophy.

/* Base (light) theme */
.custom-scrollbar {
/* Firefox */
scrollbar-width: thin; /* auto | thin | none */
scrollbar-color: var(--scrollbar-thumb) var(--scrollbar-track); /* thumb track */
/* Layout stability where supported */
scrollbar-gutter: stable both-edges; /* ignored if unsupported */
Copy link

Choose a reason for hiding this comment

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

[performance] low Priority

Issue: Potential layout thrashing with scrollbar-gutter property
Context: The scrollbar-gutter: stable both-edges property can cause layout shifts on browsers where it's not supported or has inconsistent implementation.
Suggestion: Consider testing across target browsers or add fallback handling, especially for older Safari versions.

}
.custom-scrollbar::-webkit-scrollbar {
width: var(--scrollbar-size);
height: var(--scrollbar-size);
}
.custom-scrollbar::-webkit-scrollbar-track {
background: var(--scrollbar-track);
}
.custom-scrollbar::-webkit-scrollbar-thumb {
background: var(--scrollbar-thumb);
border-radius: 999px;
border: 2px solid var(--scrollbar-track); /* visual separation from track */
}
.custom-scrollbar::-webkit-scrollbar-thumb:hover {
background: var(--scrollbar-thumb-hover); /* hover color */
}

/* Dark theme overrides (scoped to your `.dark-theme` root) */
.dark-theme .custom-scrollbar {
Copy link

Choose a reason for hiding this comment

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

[architecture] medium Priority

Issue: Redundant CSS rule duplication between theme variants
Context: The dark theme scrollbar styles are repeated both in @media query and .dark-theme class, creating maintenance overhead and potential conflicts.
Suggestion: Extract scrollbar dark theme variables into a mixin or consolidate the theme overrides into the existing .dark-theme section.

scrollbar-color: var(--scrollbar-thumb) var(--scrollbar-track); /* thumb track */
}
.dark-theme .custom-scrollbar::-webkit-scrollbar-track {
background: var(--scrollbar-track);
}
.dark-theme .custom-scrollbar::-webkit-scrollbar-thumb {
background: var(--scrollbar-thumb);
border: 2px solid var(--scrollbar-track);
}
.dark-theme .custom-scrollbar::-webkit-scrollbar-thumb:hover {
background: var(--scrollbar-thumb-hover); /* hover color */
}
}
/* =================== End Custom Scrollbar (cross-browser) =================== */

/* Everthing below here to be cleaned up over time. */

body {
Expand Down
2 changes: 1 addition & 1 deletion src/components/input/MultiSelect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ const pt = computed(() => ({
},
listContainer: () => ({
style: { maxHeight: listMaxHeight },
class: 'overflow-y-auto scrollbar-hide'
class: 'overflow-y-auto custom-scrollbar'
Copy link

Choose a reason for hiding this comment

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

[quality] low Priority

Issue: Good implementation pattern - clean class replacement
Context: The change from scrollbar-hide to custom-scrollbar maintains the same component architecture and follows Vue component best practices.
Suggestion: This is a clean implementation that properly separates concerns between styling and component logic.

Copy link

Choose a reason for hiding this comment

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

[architecture] medium Priority

Issue: Incomplete migration - other components still use scrollbar-hide
Context: A grep search reveals 8 other files still using scrollbar-hide class, creating inconsistency in scrollbar styling across the application.
Suggestion: Consider migrating all scrollbar-hide usages to custom-scrollbar for consistent styling, or provide clear guidance on when to use each approach.

}),
list: {
class: 'flex flex-col gap-0 p-0 m-0 list-none border-none text-sm'
Expand Down
2 changes: 1 addition & 1 deletion src/components/input/SingleSelect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ const pt = computed(() => ({
},
listContainer: () => ({
style: `max-height: ${listMaxHeight}`,
class: 'overflow-y-auto scrollbar-hide'
class: 'overflow-y-auto custom-scrollbar'
}),
list: {
class:
Expand Down