165 lines
14 KiB
Markdown
165 lines
14 KiB
Markdown
# Fix: GFIBER-663 — Attendance horizontal scrollbar disappears on desktop PCs
|
||
|
||
## Context
|
||
|
||
GFIBER-663 (Crítica, To Do, assigned to Alejandro Lembke): "When an admin tries to update the attendance, the horizontal scrollbar disappears. This feature works perfectly on laptops (using the trackpad), but when users are on a desktop PC using a standard mouse and want to scroll horizontally, the bar vanishes, making it impossible to navigate."
|
||
|
||
**Root cause (confirmed by code exploration):** the Attendance Grid at `/dashboard` renders through `app/(protected)/dashboard/page.tsx` → `components/dashboard/AttendanceGrid.tsx` (thin stateful wrapper) → the real markup/CSS in `packages/design-system/src/components/AttendanceGrid/index.tsx` + `style.module.scss`. The scrollable container (`.scroll`, `overflow-x/y: auto`) has **zero** scrollbar styling anywhere in the app or design-system package (confirmed by repo-wide grep — no `::-webkit-scrollbar`, `scrollbar-width`, `scrollbar-color`, or `scrollbar-gutter` rules exist, and no JS toggles scrollbar visibility). The horizontal bar is therefore the raw browser/OS-default scrollbar. On systems with auto-hiding overlay scrollbars (the Windows "Automatically hide scroll bars" setting, on by default in many configurations, plus trackpad-first macOS setups), that bar only flashes during active scroll/hover and never renders as a persistent, draggable element. Trackpad users pan via a two-finger gesture and never need to see it; desktop-mouse users have no default horizontal-scroll gesture and depend on seeing/dragging a visible bar that the app never forces to appear.
|
||
|
||
Two sibling design-system components (`DataTable`, `UserTable`) share the identical un-styled `overflow-x: auto` idiom and have the same latent bug if ever rendered wide enough to overflow — tracked as a fast-follow, not fixed here (see Scope below).
|
||
|
||
## Root cause verified in code
|
||
|
||
`packages/design-system/src/components/AttendanceGrid/style.module.scss:57-62`:
|
||
```scss
|
||
.scroll {
|
||
position: relative;
|
||
max-height: 520px;
|
||
overflow-x: auto;
|
||
overflow-y: auto;
|
||
}
|
||
```
|
||
No scrollbar-appearance rules follow it. Sticky columns inside `.scroll`: `.agentHead`/`.agentCell` (`left: 0`, 200px wide) and `.summaryHead`/`.summaryCell` (`left: 200px`, 88px wide) — line 81-182. The decorative `.scrollHint` right-edge fade (lines 199-207) only signals horizontal overflow; it is not a real scrollbar affordance and is unconditionally rendered regardless of actual overflow.
|
||
|
||
## Fix
|
||
|
||
**Chosen mechanism:** style the existing `.scroll` container with both standards-track scrollbar properties — no new elements, no JS, no `scrollbar-gutter` (rejected: it only reserves layout space for a scrollbar that already renders; it does not force an OS-overlay scrollbar to become persistently visible/draggable, and it would add an unconditional gutter that isn't accounted for in the precise 200px sticky-offset math).
|
||
|
||
- **Firefox:** `scrollbar-width: thin` + `scrollbar-color: <thumb> <track>` — setting `scrollbar-color` alone switches Firefox off the OS overlay style onto a classic, always-rendered scrollbar.
|
||
- **Chromium/WebKit (Chrome, Edge, Safari — the reported Windows/mouse environment):** declaring any `::-webkit-scrollbar` rule flips these engines from native OS overlay/auto-hide scrollbars to a classic, always-reserved, always-visible scrollbar, independent of the user's OS scrollbar setting. This is the actual fix for the reported bug.
|
||
|
||
Both axes get styled together (WebKit doesn't let the "become classic" trigger apply to only one axis without declaring size for both) — this is a deliberate, low-risk side effect that also fixes vertical-scrollbar visibility consistency; call it out in the PR description, not scope creep.
|
||
|
||
**Colors — existing tokens only** (verified present in `packages/design-system/src/tokens/tokens.css`, both `:root` and `.dark`): thumb `var(--fiber-gray-700)` (light `#5f6368`, ~6.05:1 — already the project's validated safe replacement for the previously-blocked `--fiber-gray-500`), thumb hover/active `var(--fiber-gray-900)`, track/corner `var(--fiber-gray-100)`. Dark mode needs no separate override block — the class-based `.dark` token overrides cascade automatically.
|
||
|
||
**Scope decision:** fix `AttendanceGrid` only in this change. Repo-wide grep confirmed zero `@use`/`@import` exist across any DS component's SCSS today — every stylesheet is self-contained by convention. Introducing a new shared-partial/mixin pattern would be the first use of that mechanism and needs its own validation across three build pipelines (Next `transpilePackages`, Storybook webpack, Cypress component-test webpack) — not worth the risk under a Crítica-priority hotfix. File a fast-follow ticket to mechanically copy the same rule block into `DataTable`/`UserTable`'s `.wrap` selector.
|
||
|
||
**`.scrollHint`:** keep unchanged — it signals "there's more to scroll" (a different affordance than "you can drag here") and removing it would force rewriting an existing passing Cypress test for no benefit in a hotfix.
|
||
|
||
### Exact CSS — `packages/design-system/src/components/AttendanceGrid/style.module.scss`
|
||
|
||
Append after the existing `.scroll { ... }` block (lines 57-62):
|
||
|
||
```scss
|
||
.scroll {
|
||
position: relative;
|
||
max-height: 520px;
|
||
overflow-x: auto;
|
||
overflow-y: auto;
|
||
|
||
// Force a persistently visible, styled scrollbar instead of relying on the
|
||
// browser/OS default. Auto-hiding overlay scrollbars (Windows "Automatically
|
||
// hide scroll bars", trackpad-first macOS) never render a draggable bar for
|
||
// desktop-mouse users, who have no horizontal-scroll gesture. GFIBER-663.
|
||
scrollbar-width: thin;
|
||
scrollbar-color: var(--fiber-gray-700) var(--fiber-gray-100);
|
||
}
|
||
|
||
// Chromium/WebKit: declaring ::-webkit-scrollbar switches the element from
|
||
// native OS overlay scrollbars to a classic, always-reserved, always-visible
|
||
// scrollbar — the actual fix for the reported Windows/Chrome bug.
|
||
.scroll::-webkit-scrollbar {
|
||
width: 10px;
|
||
height: 10px;
|
||
}
|
||
|
||
.scroll::-webkit-scrollbar-track {
|
||
background-color: var(--fiber-gray-100);
|
||
}
|
||
|
||
.scroll::-webkit-scrollbar-thumb {
|
||
background-color: var(--fiber-gray-700);
|
||
border-radius: var(--radius-input);
|
||
}
|
||
|
||
.scroll::-webkit-scrollbar-thumb:hover,
|
||
.scroll::-webkit-scrollbar-thumb:active {
|
||
background-color: var(--fiber-gray-900);
|
||
}
|
||
|
||
.scroll::-webkit-scrollbar-corner {
|
||
background-color: var(--fiber-gray-100);
|
||
}
|
||
```
|
||
|
||
## Tests (TDD — write these first, they should fail against current CSS, then pass after the fix)
|
||
|
||
**Do not touch `AttendanceGrid.test.tsx` (Jest).** Verified: `packages/design-system/jest.config.js` maps `.scss` to `identity-obj-proxy`, so Jest/RTL never loads real CSS — `getComputedStyle` assertions there would be meaningless (pass/fail independent of the actual fix).
|
||
|
||
**Add to `AttendanceGrid.test.cy.tsx`**, inside/after the existing `describe('AttendanceGrid (component) — scroll & sticky'` block (reuses `FULL_MONTH`/`fullGet` fixtures already defined there, lines 54-64) — Cypress runs real Chromium with the real compiled SCSS, so `getComputedStyle` reflects the actual fix:
|
||
|
||
1. Assert `::-webkit-scrollbar` width/height (10px) and a non-transparent thumb background — proves the classic scrollbar is forced.
|
||
2. Assert `overflowX`/`overflowY` remain `auto` — proves the fix didn't change overflow behavior.
|
||
3. New vertical-scroll regression (none of today's fixtures force vertical overflow — today's `AGENTS` array only has 2 entries): add a local ~15-agent fixture, mount, assert `scrollHeight > clientHeight`, scroll to bottom, confirm the last agent is visible AND the agent-name header (`.agentHead`) stays pinned — proves the scrollbar fix didn't break sticky columns or vertical scroll.
|
||
|
||
**Add to `AttendanceGrid.stories.tsx`:** a `DenseGridBothScrollbars` story (15 agents × the existing 31-day `FULL_MONTH`, already defined above `FullMonthScroll`) forcing both scrollbars simultaneously — the manual-QA scenario, following the same precedent as the `BarChart` `DenseDailySeries` regression story used for a prior overflow hotfix in this project.
|
||
|
||
## Manual verification (cannot be automated — no headless engine validates real OS scrollbar chrome/paint)
|
||
|
||
- Windows 10/11 + Chrome/Edge with "Automatically hide scroll bars in Windows" **on** (the exact reported condition) — confirm the bar is persistently visible without hover, and mouse-draggable.
|
||
- macOS Safari/Chrome with trackpad-based auto-hide scrollbars — confirm no regression for the working trackpad case.
|
||
- Firefox (Windows/macOS) — confirm `scrollbar-color` renders and is draggable.
|
||
- Both light and dark Storybook themes (`DenseGridBothScrollbars` story) — confirm thumb/track contrast.
|
||
- Sticky agent/summary columns stay glued while horizontal-dragging in a real browser.
|
||
|
||
## Files to touch
|
||
|
||
- `packages/design-system/src/components/AttendanceGrid/style.module.scss` — the CSS fix (only production-code change).
|
||
- `packages/design-system/src/components/AttendanceGrid/AttendanceGrid.test.cy.tsx` — 3 new Cypress assertions (webkit-scrollbar, overflow-auto, vertical-scroll-unaffected).
|
||
- `packages/design-system/src/components/AttendanceGrid/AttendanceGrid.stories.tsx` — `DenseGridBothScrollbars` story.
|
||
- Not touched now, tracked as fast-follow: `packages/design-system/src/components/DataTable/style.module.scss`, `packages/design-system/src/components/UserTable/style.module.scss`.
|
||
|
||
## Verification (end-to-end)
|
||
|
||
1. `npm run test --workspace=@gfiber/design-system` (Jest) — unaffected, still green (no changes to `.test.tsx`).
|
||
2. Cypress component tests for `AttendanceGrid` — new assertions fail on `main` (pre-fix) and pass after the SCSS change.
|
||
3. `npm run storybook --workspace=@gfiber/design-system` → open `DenseGridBothScrollbars`, toggle light/dark — visually confirm persistent, styled scrollbar with adequate contrast, sticky columns intact.
|
||
4. Manual checklist above (Windows/Chrome is the priority — it's the reported environment) before closing the ticket as Crítica.
|
||
5. CI: `warden`/`harden`/`jorden` (design/a11y review) must pass — this change deliberately reuses only already-AA-validated tokens to avoid a repeat of past `jorden`-blocked contrast issues.
|
||
|
||
## Execution — delegated to a background agent (`background-orchestrator` skill)
|
||
|
||
Per the user's request, this plan is not executed directly in this session. It hands off to the `background-orchestrator` skill, following the same pattern already used for prior tickets in this repo (see Docmost `Agentes_Activos`: `agente-gfiber-649-pending`, `agente-gfiber-658-is-enabled`, etc.):
|
||
|
||
1. Create a dedicated git worktree + branch (e.g. `agente-gfiber-663-attendance-scrollbar`) off `dev`, per the project's branch-per-feature convention.
|
||
2. Launch an autonomous agent in tmux inside that worktree, briefed with this plan verbatim: write the failing Cypress tests first (TDD — they must fail against the current, un-styled `.scroll` CSS), then the SCSS fix, then the Storybook story, then run the full verification checklist above.
|
||
3. Monitor the agent, answer any blocking questions it raises.
|
||
4. On completion, the agent opens a PR to `dev` via the `aleleba-pr` skill — **never auto-merges**.
|
||
5. Record the agent's lifecycle (start, ticket, status, PR link) on the Docmost `Agentes_Activos` page, matching the existing history format.
|
||
|
||
This session's job ends at handing off a decision-complete plan; the orchestrator skill owns spawning, monitoring, and PR creation from here.
|
||
|
||
---
|
||
|
||
# Fix: docmost-context SessionStart hook not triggering
|
||
|
||
## Context
|
||
|
||
The original ask ("¿puedes leer de Jira el ticket GFIBER-663?") was a read-only lookup — already completed. `GFIBER-663` ("Admin Attendance page: Horizontal scrollbar disappears on desktop PCs", Crítica, To Do, assigned to Alejandro Lembke) was fetched via the `atlassian` MCP and reported above.
|
||
|
||
The user then raised a separate, real problem: the global `docmost-context` skill — which is supposed to auto-load project context from Docmost at the start of every conversation via a `SessionStart` hook — appears to never fire. This matters because `CLAUDE.md` and the `docmost-context` skill both assume this happens automatically; if it silently doesn't, every session starts without Docmost context and nobody notices until something is missed.
|
||
|
||
## Root cause
|
||
|
||
Found by inspecting `~/.claude/settings.json` and the hook script directly (read-only):
|
||
|
||
- `~/.claude/settings.json` correctly registers the hook for `SessionStart` on matchers `startup`, `resume`, and `clear`, pointing at `/home/aleleba/.claude/hooks/docmost-session-start.sh`.
|
||
- The script itself is correct — it detects the git project name and instructs the model to invoke the `docmost-context` skill.
|
||
- **The file is not executable**: `ls -la` shows `-rw-rw-rw-` (no `x` bit) on `/home/aleleba/.claude/hooks/docmost-session-start.sh`. Claude Code's hook runner invokes the command path directly; without the execute bit, the OS refuses to run it (`Permission denied`), so the hook silently produces no output and the model never sees the instruction to load Docmost context. This fully explains "la skill global la estás ignorando, ¿el hook no se hace trigger?" — it's not a skill-detection issue, it's a filesystem permissions issue on the hook script.
|
||
|
||
## Fix
|
||
|
||
Single command, no code change:
|
||
|
||
```bash
|
||
chmod +x /home/aleleba/.claude/hooks/docmost-session-start.sh
|
||
```
|
||
|
||
This is a global (`~/.claude`) file, not part of the `gfiber-pilot-extension` repo — no commit/PR involved.
|
||
|
||
## Verification
|
||
|
||
1. `ls -la /home/aleleba/.claude/hooks/docmost-session-start.sh` → confirm `x` bits present (e.g. `-rwxrwxrwx` or `-rwxr-xr-x`).
|
||
2. Start a fresh Claude Code session (or `/clear`) inside a git repo → confirm the `[docmost-context] Nueva conversación detectada...` message appears in the hook output, and that the model subsequently invokes the `docmost-context` skill before responding to the first user message.
|
||
3. Optionally run the hook manually to sanity-check output before relying on the harness: `bash /home/aleleba/.claude/hooks/docmost-session-start.sh` from within this repo — should print the instructional block referencing `gfiber-pilot-extension`.
|