fix(css): shelf kebab dropdown unstyled regression (bookshelf-2jxl) #314

Merged

zombor merged 1 commit from bd-bookshelf-2jxl into main 2026-06-03 16:05:12 +00:00

Conversation 2 Commits 1 Files changed 2 +46 -2

zombor commented 2026-06-03 15:49:45 +00:00

Owner

Copy link

Summary

• Adds .shelf-kebab-menu to the existing grouped CSS rules for the kebab dropdown panel (display:none base + display:block .is-open). This fixes the production regression from bd4l (#309) where the shelf dropdown was always-visible and unpositioned.
• No duplication — the two rules now have comma-separated selectors sharing the same bodies.
• Strengthens the e2e/browser/shelves_test.go shelf kebab tests: adds two new It blocks asserting the dropdown is display:none on initial load and display:block after clicking the button. These assertions fail without the CSS fix and pass with it.

Test plan

• make test — unit tests green (no Go changes, CSS/e2e only)
• make e2e — new visibility assertions pass; existing kebab tests pass
• Navigate to /books with a shelf in sidebar — confirm Edit/Delete NOT visible until kebab clicked
• Click kebab for a shelf — confirm dropdown appears correctly positioned

Closes bead bookshelf-2jxl on merge.

## Summary - Adds `.shelf-kebab-menu` to the existing grouped CSS rules for the kebab dropdown panel (`display:none` base + `display:block .is-open`). This fixes the production regression from bd4l (#309) where the shelf dropdown was always-visible and unpositioned. - No duplication — the two rules now have comma-separated selectors sharing the same bodies. - Strengthens the `e2e/browser/shelves_test.go` shelf kebab tests: adds two new `It` blocks asserting the dropdown is `display:none` on initial load and `display:block` after clicking the button. These assertions **fail** without the CSS fix and **pass** with it. ## Test plan - [ ] `make test` — unit tests green (no Go changes, CSS/e2e only) - [ ] `make e2e` — new visibility assertions pass; existing kebab tests pass - [ ] Navigate to /books with a shelf in sidebar — confirm Edit/Delete NOT visible until kebab clicked - [ ] Click kebab for a shelf — confirm dropdown appears correctly positioned Closes bead bookshelf-2jxl on merge.

zombor added 1 commit 2026-06-03 15:49:45 +00:00

fix(css): add .shelf-kebab-menu to kebab dropdown rules; strengthen e2e visibility assertions ... 94fad60bec

REGRESSION from bd4l (#309): shelf kebab dropdown was always-visible and
unpositioned because .shelf-kebab-menu had no CSS rules (only
.library-kebab-menu was styled). Group .shelf-kebab-menu into the two
existing panel rules (display:none base + display:block .is-open) so both
share the same styles without duplication.

Strengthen the browser e2e for the shelf kebab: add two assertions — one
that the panel is display:none on initial load, and one that it becomes
display:block after clicking the kebab button. These fail without the CSS
fix and pass with it.

Closes bead bookshelf-2jxl on merge.

zombor commented 2026-06-03 15:57:19 +00:00

Author

Owner

Copy link

SECURITY REVIEW — bookshelf-2jxl (PR #314)

Scope: CSS selector expansion (static/css/main.css) + two new e2e browser assertions (e2e/browser/shelves_test.go). No Go handler, template, middleware, auth, or data-access changes.

Analysis

static/css/main.css
The change groups .shelf-kebab-menu into the existing .library-kebab-menu hide/show rules (lines 487–503). This is a purely presentational change:

• No new HTML is introduced; the .shelf-kebab-menu element already exists in the DOM (rendered by the Stimulus controller wired in a prior PR).
• display: none is the default — hiding is the secure state. The is-open class that reveals the menu is only applied by the Stimulus controller on an explicit user click on the kebab button, not via any URL parameter, cookie, or server-injected state.
• CSS alone cannot bypass authentication, authorization, or CSRF protection. Visibility rules have zero influence on which API endpoints are reachable.
• No new CSS custom properties, attr(), url(), or content: values are introduced that could serve as an injection surface.

e2e/browser/shelves_test.go
Two new It blocks verify the hide-on-load / show-on-click behavior:

• Both tests authenticate as the test user via loginPage(page) before touching any shelf UI — no unauthenticated surface is exercised.
• The selector used to find the kebab button is scoped to a specific shelf's data-* attribute value derived from a just-created shelf ID; no user-controlled input flows into the selector string at runtime.
• Eventually polling with a 2s/50ms budget is safe; no goroutine leak or resource exhaustion possible in a test context.
• No fixture files, secrets, or environment variables are added.

Findings

No security findings.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## SECURITY REVIEW — bookshelf-2jxl (PR #314) **Scope:** CSS selector expansion (`static/css/main.css`) + two new e2e browser assertions (`e2e/browser/shelves_test.go`). No Go handler, template, middleware, auth, or data-access changes. ### Analysis **static/css/main.css** The change groups `.shelf-kebab-menu` into the existing `.library-kebab-menu` hide/show rules (lines 487–503). This is a purely presentational change: - No new HTML is introduced; the `.shelf-kebab-menu` element already exists in the DOM (rendered by the Stimulus controller wired in a prior PR). - `display: none` is the default — hiding is the *secure* state. The `is-open` class that reveals the menu is only applied by the Stimulus controller on an explicit user click on the kebab button, not via any URL parameter, cookie, or server-injected state. - CSS alone cannot bypass authentication, authorization, or CSRF protection. Visibility rules have zero influence on which API endpoints are reachable. - No new CSS custom properties, `attr()`, `url()`, or `content:` values are introduced that could serve as an injection surface. **e2e/browser/shelves_test.go** Two new `It` blocks verify the hide-on-load / show-on-click behavior: - Both tests authenticate as the test user via `loginPage(page)` before touching any shelf UI — no unauthenticated surface is exercised. - The selector used to find the kebab button is scoped to a specific shelf's `data-*` attribute value derived from a just-created shelf ID; no user-controlled input flows into the selector string at runtime. - `Eventually` polling with a 2s/50ms budget is safe; no goroutine leak or resource exhaustion possible in a test context. - No fixture files, secrets, or environment variables are added. ### Findings No security findings. REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor commented 2026-06-03 15:57:36 +00:00

Author

Owner

Copy link

Code Review — bookshelf-2jxl (PR #314)

Phase 0: DEMO Verification

No DEMO block in the bead. The fix is a CSS regression repair — the correct verification is the two new e2e tests. Both were inspected in the diff; CI is green (per brief).

Phase 1: Spec Compliance

Spec: group .shelf-kebab-menu into the existing display:none / display:block rules; add e2e assertions for hidden-by-default and shown-on-click.

• static/css/main.css:490 — .library-kebab-menu, .shelf-kebab-menu { display: none; ... }. Comma-grouped correctly. No rule body duplication; the full shared block (position, z-index, min-width, background, border, border-radius, box-shadow, padding) is declared once.
• static/css/main.css:504 — .library-kebab-menu.is-open, .shelf-kebab-menu.is-open { display: block; }. Correct counterpart.
• e2e/browser/shelves_test.go:262 — "hidden on load" test: navigates to /books, calls getComputedStyle(this).display on .shelf-kebab-menu, expects "none". Passes only if CSS rule is present.
• e2e/browser/shelves_test.go:280 — "visible after click" test: creates a shelf, navigates to /shelves/{id}, clicks the kebab, polls getComputedStyle with Eventually until "block". Correctly handles CSS transition timing. Would fail without the .is-open display rule.

Shelf menu item buttons use .library-kebab-menu__item (base.html:226,228,258,260) — those styles are unchanged and continue to apply.

No other kebab/menu styles are touched.

Phase 2: Code Quality

One observation worth noting:

[MINOR] e2e/browser/shelves_test.go:262 — "hidden on load" test navigates to /books, not /shelves/{id}
The test uses page.MustElement(".shelf-kebab-menu") which finds the first match anywhere in the DOM. This works (sidebar is always rendered), but is slightly asymmetric with the click test which navigates to the shelf's own page. Not a correctness issue — both routes render the sidebar containing .shelf-kebab-menu.

[MINOR] e2e/browser/shelves_test.go:254-295 — new It blocks are not wrapped in a Describe/Context grouping
The two new tests sit directly inside the outer kebab Describe alongside the older "kebab menu has Edit and Delete items" It. Convention in this file uses separate Describe blocks per concern. Minor structural nit; no behavioral impact.

REVIEW VERDICT: 0 blocker, 0 major, 2 minor

## Code Review — bookshelf-2jxl (PR #314) ### Phase 0: DEMO Verification No DEMO block in the bead. The fix is a CSS regression repair — the correct verification is the two new e2e tests. Both were inspected in the diff; CI is green (per brief). ### Phase 1: Spec Compliance Spec: group `.shelf-kebab-menu` into the existing `display:none` / `display:block` rules; add e2e assertions for hidden-by-default and shown-on-click. - `static/css/main.css:490` — `.library-kebab-menu, .shelf-kebab-menu { display: none; ... }`. Comma-grouped correctly. No rule body duplication; the full shared block (position, z-index, min-width, background, border, border-radius, box-shadow, padding) is declared once. - `static/css/main.css:504` — `.library-kebab-menu.is-open, .shelf-kebab-menu.is-open { display: block; }`. Correct counterpart. - `e2e/browser/shelves_test.go:262` — "hidden on load" test: navigates to `/books`, calls `getComputedStyle(this).display` on `.shelf-kebab-menu`, expects `"none"`. Passes only if CSS rule is present. - `e2e/browser/shelves_test.go:280` — "visible after click" test: creates a shelf, navigates to `/shelves/{id}`, clicks the kebab, polls `getComputedStyle` with `Eventually` until `"block"`. Correctly handles CSS transition timing. Would fail without the `.is-open` display rule. Shelf menu item buttons use `.library-kebab-menu__item` (base.html:226,228,258,260) — those styles are unchanged and continue to apply. No other kebab/menu styles are touched. ### Phase 2: Code Quality One observation worth noting: [MINOR] e2e/browser/shelves_test.go:262 — "hidden on load" test navigates to `/books`, not `/shelves/{id}` The test uses `page.MustElement(".shelf-kebab-menu")` which finds the first match anywhere in the DOM. This works (sidebar is always rendered), but is slightly asymmetric with the click test which navigates to the shelf's own page. Not a correctness issue — both routes render the sidebar containing `.shelf-kebab-menu`. [MINOR] e2e/browser/shelves_test.go:254-295 — new `It` blocks are not wrapped in a `Describe`/`Context` grouping The two new tests sit directly inside the outer kebab `Describe` alongside the older "kebab menu has Edit and Delete items" `It`. Convention in this file uses separate `Describe` blocks per concern. Minor structural nit; no behavioral impact. REVIEW VERDICT: 0 blocker, 0 major, 2 minor

zombor merged commit 09fd6e5d27 into main 2026-06-03 16:05:12 +00:00

zombor referenced this pull request from a commit 2026-06-03 16:05:14 +00:00

Merge pull request 'fix(css): shelf kebab dropdown unstyled regression (bookshelf-2jxl)' (#314) from bd-bookshelf-2jxl into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!314

No description provided.