Skip to content

Commit e78ab36

Browse files
Copilottyler-dane
andcommitted
Replace useEffect approach with CSS-only scrollbar styling and add tests
Co-authored-by: tyler-dane <30163055+tyler-dane@users.noreply.github.com>
1 parent d94dbbf commit e78ab36

File tree

3 files changed

+168
-55
lines changed

3 files changed

+168
-55
lines changed

packages/web/src/components/GlobalStyle/styled.ts

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,33 +25,29 @@ export const GlobalStyle = createGlobalStyle`
2525
/* Command Palette Scrollbar Styling */
2626
.command-palette .overflow-y-auto {
2727
/* Always reserve scrollbar space */
28-
scrollbar-width: auto;
28+
scrollbar-width: thin;
29+
scrollbar-color: ${({ theme }) => theme.color.panel.scrollbar} transparent;
2930
3031
/* Webkit scrollbar styling (Chrome, Safari, Edge) */
3132
&::-webkit-scrollbar {
3233
width: 8px;
3334
}
3435
35-
/* Hide the scrollbar thumb by default (transparent) */
36-
&::-webkit-scrollbar-thumb {
37-
background-color: transparent;
38-
border-radius: ${({ theme }) => theme.shape.borderRadius};
39-
transition: background-color 0.3s ease;
36+
/* Scrollbar track - always visible but subtle */
37+
&::-webkit-scrollbar-track {
38+
background: transparent;
4039
}
4140
42-
/* On hover of the container, make the scrollbar thumb visible */
43-
&:hover::-webkit-scrollbar-thumb {
41+
/* Scrollbar thumb - visible with theme colors */
42+
&::-webkit-scrollbar-thumb {
4443
background-color: ${({ theme }) => theme.color.panel.scrollbar};
44+
border-radius: ${({ theme }) => theme.shape.borderRadius};
45+
transition: background-color 0.3s ease;
4546
}
4647
4748
/* On hover of the thumb itself, make it more visible */
48-
&:hover::-webkit-scrollbar-thumb:hover {
49+
&::-webkit-scrollbar-thumb:hover {
4950
background-color: ${({ theme }) => theme.color.panel.scrollbarActive};
5051
}
51-
52-
/* Show scrollbar during active scrolling */
53-
&.scrolling::-webkit-scrollbar-thumb {
54-
background-color: ${({ theme }) => theme.color.panel.scrollbar};
55-
}
5652
}
5753
`;
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import React from "react";
2+
import "@testing-library/jest-dom";
3+
import { screen } from "@testing-library/react";
4+
import { render } from "@web/__tests__/__mocks__/mock.render";
5+
6+
// Mock the react-cmdk library
7+
jest.mock("react-cmdk", () => {
8+
const MockCommandPalette = ({ children, isOpen }: any) => (
9+
<div
10+
className="command-palette"
11+
data-testid="command-palette"
12+
style={{ display: isOpen ? "block" : "none" }}
13+
>
14+
<div className="overflow-y-auto" data-testid="scrollable-content">
15+
{children}
16+
</div>
17+
</div>
18+
);
19+
20+
MockCommandPalette.Page = ({ children }: any) => <div>{children}</div>;
21+
MockCommandPalette.List = ({ children }: any) => <div>{children}</div>;
22+
MockCommandPalette.ListItem = ({ children }: any) => <div>{children}</div>;
23+
MockCommandPalette.FreeSearchAction = () => <div>No results</div>;
24+
25+
return {
26+
__esModule: true,
27+
default: MockCommandPalette,
28+
filterItems: jest.fn((items) => items),
29+
getItemIndex: jest.fn(() => 0),
30+
useHandleOpenCommandPalette: jest.fn(),
31+
};
32+
});
33+
34+
// Mock the necessary modules
35+
jest.mock("@web/store/store.hooks", () => ({
36+
useAppDispatch: () => jest.fn(),
37+
useAppSelector: (selector: any) => {
38+
if (selector.toString().includes("selectIsCmdPaletteOpen")) {
39+
return true; // Make the command palette open for testing
40+
}
41+
return false;
42+
},
43+
}));
44+
45+
const CmdPalette = require("./CmdPalette").default;
46+
47+
const mockProps = {
48+
today: {
49+
format: jest.fn(() => "Monday, January 1"),
50+
} as any,
51+
isCurrentWeek: true,
52+
startOfView: new Date(),
53+
endOfView: new Date(),
54+
util: {
55+
goToToday: jest.fn(),
56+
} as any,
57+
scrollUtil: {
58+
scrollToNow: jest.fn(),
59+
} as any,
60+
};
61+
62+
describe("CmdPalette Scrollbar", () => {
63+
beforeEach(() => {
64+
jest.clearAllMocks();
65+
});
66+
67+
it("renders command palette with scrollable content", () => {
68+
render(<CmdPalette {...mockProps} />);
69+
70+
const commandPalette = screen.getByTestId("command-palette");
71+
const scrollableContent = screen.getByTestId("scrollable-content");
72+
73+
expect(commandPalette).toBeInTheDocument();
74+
expect(scrollableContent).toBeInTheDocument();
75+
});
76+
77+
it("applies correct CSS classes for scrollbar styling", () => {
78+
render(<CmdPalette {...mockProps} />);
79+
80+
const commandPalette = screen.getByTestId("command-palette");
81+
const scrollableContent = screen.getByTestId("scrollable-content");
82+
83+
expect(commandPalette).toHaveClass("command-palette");
84+
expect(scrollableContent).toHaveClass("overflow-y-auto");
85+
});
86+
87+
it("scrollbar styling is applied through GlobalStyle", () => {
88+
render(<CmdPalette {...mockProps} />);
89+
90+
// Check that the GlobalStyle component is rendered
91+
// The actual scrollbar styling is tested through CSS-in-JS
92+
const scrollableContent = screen.getByTestId("scrollable-content");
93+
94+
// Verify the element has the correct class that targets our CSS
95+
expect(scrollableContent).toHaveClass("overflow-y-auto");
96+
97+
// The scrollbar styling itself is handled by CSS and cannot be directly
98+
// tested in jsdom, but we can verify the structure is correct
99+
const commandPalette = scrollableContent.closest(".command-palette");
100+
expect(commandPalette).toBeInTheDocument();
101+
});
102+
103+
it("maintains accessibility for scrollable content", () => {
104+
render(<CmdPalette {...mockProps} />);
105+
106+
const scrollableContent = screen.getByTestId("scrollable-content");
107+
108+
// Verify the scrollable content is accessible
109+
expect(scrollableContent).toBeInTheDocument();
110+
expect(scrollableContent).toHaveClass("overflow-y-auto");
111+
112+
// The scrollbar should not interfere with keyboard navigation
113+
// This is ensured by using standard overflow CSS properties
114+
});
115+
116+
describe("CSS Scrollbar Properties", () => {
117+
it("uses theme colors for scrollbar styling", () => {
118+
render(<CmdPalette {...mockProps} />);
119+
120+
// The GlobalStyle component should be present to apply scrollbar styles
121+
// We can't directly test CSS-in-JS styles in jsdom, but we verify
122+
// the component structure supports the styling
123+
const scrollableContent = screen.getByTestId("scrollable-content");
124+
const commandPalette = scrollableContent.closest(".command-palette");
125+
126+
expect(commandPalette).toBeInTheDocument();
127+
expect(scrollableContent).toHaveClass("overflow-y-auto");
128+
});
129+
130+
it("provides proper scrollbar width and styling", () => {
131+
render(<CmdPalette {...mockProps} />);
132+
133+
const scrollableContent = screen.getByTestId("scrollable-content");
134+
135+
// Verify the element structure that our CSS targets
136+
expect(scrollableContent).toHaveClass("overflow-y-auto");
137+
138+
// The CSS rules are:
139+
// - 8px width for webkit scrollbar
140+
// - Theme-based colors for thumb
141+
// - Hover states for better UX
142+
// These are applied through the GlobalStyle component
143+
});
144+
});
145+
146+
describe("Cross-browser Compatibility", () => {
147+
it("supports both webkit and standard scrollbar properties", () => {
148+
render(<CmdPalette {...mockProps} />);
149+
150+
const scrollableContent = screen.getByTestId("scrollable-content");
151+
152+
// Our CSS implementation supports:
153+
// - webkit browsers (Chrome, Safari, Edge) with ::-webkit-scrollbar
154+
// - Firefox with scrollbar-width and scrollbar-color
155+
expect(scrollableContent).toHaveClass("overflow-y-auto");
156+
});
157+
});
158+
});

packages/web/src/views/CmdPalette/CmdPalette.tsx

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -53,47 +53,6 @@ const CmdPalette = ({
5353
setOpen(_open);
5454
}, [_open]);
5555

56-
// Handle scrollbar visibility during scrolling
57-
useEffect(() => {
58-
if (!open) return;
59-
60-
const handleScroll = () => {
61-
const scrollableElement = document.querySelector(
62-
".command-palette .overflow-y-auto",
63-
);
64-
if (scrollableElement) {
65-
scrollableElement.classList.add("scrolling");
66-
67-
// Clear any existing timeout
68-
const existingTimeout = (scrollableElement as any).scrollTimeout;
69-
if (existingTimeout) {
70-
clearTimeout(existingTimeout);
71-
}
72-
73-
// Remove the scrolling class after scrolling stops
74-
(scrollableElement as any).scrollTimeout = setTimeout(() => {
75-
scrollableElement.classList.remove("scrolling");
76-
}, 150);
77-
}
78-
};
79-
80-
const scrollableElement = document.querySelector(
81-
".command-palette .overflow-y-auto",
82-
);
83-
84-
if (scrollableElement) {
85-
scrollableElement.addEventListener("scroll", handleScroll);
86-
87-
return () => {
88-
scrollableElement.removeEventListener("scroll", handleScroll);
89-
const timeout = (scrollableElement as any).scrollTimeout;
90-
if (timeout) {
91-
clearTimeout(timeout);
92-
}
93-
};
94-
}
95-
}, [open]);
96-
9756
useHandleOpenCommandPalette(setOpen);
9857

9958
const handleCreateSomedayDraft = async (

0 commit comments

Comments
 (0)