fix(landing): scrollbars hide when inactive#94
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR adds interactive scrollbar visibility to the landing page. CSS introduces new scrollbar design tokens for colors and styling rules (base appearance, active state, and WebKit specifics). JavaScript listens to user interactions (scroll, wheel, touch, pointer, keyboard) and briefly displays scrollbars by toggling a ChangesInteractive Scrollbar Visibility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Preview DeployedYour changes have been deployed to a preview environment: 🌐 Landing Page: https://pr-94.preview.caplets.dev Built from commit 6d15eb3 🤖 This comment updates automatically with each push. |
|
| Filename | Overview |
|---|---|
| apps/landing/src/pages/index.astro | Adds a JS debounce-based mechanism to temporarily reveal scrollbars on user interaction (scroll, wheel, touch, pointer, keyboard); logic is straightforward and correctly uses passive event listeners. |
| apps/landing/src/styles/global.css | Introduces CSS variables for scrollbar theming and applies them via both the standard scrollbar-color (on *) and WebKit pseudo-elements; one redundant selector present in the hover/focus activation rule. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User Interaction] --> B{Event type?}
B -->|scroll / wheel / touchmove| C[showScrollbarsBriefly]
B -->|pointerdown| C
B -->|keydown| D{Scroll key?}
D -->|ArrowUp/Down, Home/End\nPageUp/Down, Space| C
D -->|Other key| E[No-op]
C --> F[Add 'scrollbars-active' to html]
C --> G[clearTimeout previous timer]
G --> H[setTimeout 900ms]
H --> I[Remove 'scrollbars-active' from html]
F --> J[CSS: --scrollbar-thumb = active color]
I --> K[CSS: --scrollbar-thumb = transparent]
J --> L[Scrollbars visible on all elements]
K --> M[Scrollbars hidden]
N[Hover .agent-setup-panel] --> O[CSS: --scrollbar-thumb = active on panel only]
P[focus-within .agent-setup-panel] --> O
Reviews (1): Last reviewed commit: "fix(landing): scrollbars hide when inact..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| html.scrollbars-active, | ||
| :where(.agent-setup-panel, .agent-setup-panel pre):hover, | ||
| :where(.agent-setup-panel, .agent-setup-panel pre):focus-within { | ||
| --scrollbar-thumb: var(--scrollbar-thumb-active); | ||
| } |
There was a problem hiding this comment.
:where(.agent-setup-panel pre) in hover/focus selector
In CSS, :hover on a child element propagates up the ancestor chain, so hovering .agent-setup-panel pre also triggers :hover on the parent .agent-setup-panel. The --scrollbar-thumb variable then inherits down to the pre, making the :where(.agent-setup-panel pre):hover and :where(.agent-setup-panel pre):focus-within selectors unreachable in any meaningful way that the .agent-setup-panel variants don't already cover. The rule could be simplified to just :where(.agent-setup-panel):hover and :where(.agent-setup-panel):focus-within.
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!
| window.addEventListener("keydown", (event: KeyboardEvent) => { | ||
| if (["ArrowDown", "ArrowUp", "End", "Home", "PageDown", "PageUp", "Space"].includes(event.code)) { | ||
| showScrollbarsBriefly(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The anonymous arrow function passed to window.addEventListener("keydown", ...) has no stored reference, so it cannot be passed to removeEventListener later. The other four listeners all use the named showScrollbarsBriefly reference and can be cleaned up. If Astro View Transitions are ever added to this page, navigating back and forth would stack up duplicate keydown listeners on window, causing the scrollbar to re-appear multiple times per keypress. Extracting the arrow function into a named handler (or the same showScrollbarsBriefly with a guard, since all the listed keys trigger page scrolling anyway) would make cleanup straightforward.
1 participant
This pull request enhances the visual behavior and accessibility of scrollbars on the landing page by introducing dynamic styling and interaction feedback. The main improvements include showing custom-styled scrollbars briefly during user interaction and updating the global CSS to support these visual changes.
Scrollbar interaction and behavior improvements:
apps/landing/src/pages/index.astro)Styling and theming updates:
apps/landing/src/styles/global.css)apps/landing/src/styles/global.css)scrollbars-activeclass on thehtmlelement and certain panels to trigger the active scrollbar style, improving visibility during user interaction. (apps/landing/src/styles/global.css)Summary by CodeRabbit
Release Notes