fix(planning): scope 5-min-label timepicker CSS to the minute face#1627
Merged
Conversation
Design for scoping the one-minute 5-min-label CSS to the minute face only, so the feature (#1625) stops hiding hours 2-5,7-10,12 on the hours dial. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The one-minute 5-min-label feature (#1625) hid most HOUR labels (showed only 1, 6, 11) because its selector keys on .clock-face__number--outer — a class ngx-material-timepicker uses for BOTH the 12-cell hours ring and the 60-cell minute ring — applied on the persistent overlay root. On the hours dial :nth-child(5n+1) kept only positions 1/6/11. Scope the thinning to the minute face only via `.clock-face__container:has(.clock-face__number--outer:nth-child(13))`: the minute container has a 13th outer cell, the hours container never does (its 13th child in 24h mode is .clock-face__inner, a different class), so the rule can no longer touch the hours dial. Minute behavior is unchanged. Every minute stays selectable (visibility:hidden keeps click targets). Harden the Playwright spec to assert all 12 hour labels are visible while the hours dial is open (before the hour click) — the gap that let #1625 pass CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the planning page’s ngx-material-timepicker one-minute mode where the “hide non-5-minute labels” CSS unintentionally also hid most hour labels due to shared DOM classes across the hours and minutes dials.
Changes:
- Scoped the “hide non-5-minute labels” rule so it only applies on the minute face (via a
:has()gate). - Hardened the Playwright e2e test to assert all 12 hour labels are visible before switching to the minute dial.
- Added a design/spec doc capturing the root cause and approach.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-plannings-table/time-plannings-table.component.scss | Scopes the thinning CSS to the minute dial using a :has()-based predicate so hours aren’t affected. |
| eform-client/playwright/e2e/plugins/time-planning-pn/b1m/dashboard-edit-a.spec.ts | Adds a regression assertion that the hours dial keeps all 12 labels visible before validating minute-label thinning. |
| docs/superpowers/specs/2026-06-25-timepicker-hours-dial-scope-fix-design.md | Documents the regression, requirements, and the scoping approach for future reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Timepicker hours-dial regression — scoped 5-minute-label fix | ||
|
|
||
| **Date:** 2026-06-25 | ||
| **Status:** design approved, pending implementation |
Comment on lines
+37
to
+39
| .timepicker.timepicker--hide-non-multiples-of-5 | ||
| .clock-face:has(.clock-face__number--outer:nth-child(13)) | ||
| .clock-face__number--outer:not(:nth-child(5n+1)) > span { visibility: hidden; } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The one-minute 5-min-label timepicker feature (#1625, live on
stable) breaks the hours dial: it shows only hours 1, 6, 11, hiding the rest.Root cause
The thinning rule
keys on
.clock-face__number--outer— butngx-material-timepickerv13.1.1 uses that same class for both the 12-cell hours ring and the 60-cell minute ring, and the.timepicker--hide-non-multiples-of-5class sits on the persistent overlay root present on both faces. On the 12-cell hours dial,:nth-child(5n+1)keeps positions 1/6/11 → hours 1, 6, 11. It only works on minutes by coincidence of count. The CI test missed it because it asserted label visibility after clicking an hour (already on the minute face).Fix
.clock-face__container:has(.clock-face__number--outer:nth-child(13)). The minute container has a 13th--outercell; the hours container never does (its 13th child in 24h mode is.clock-face__inner, a different class), so the rule can no longer match the hours dial. Verified against the compiled v13.1.1 template; minute behavior is byte-identical; every minute stays selectable (visibility:hiddenkeeps click targets).Kept in the
ViewEncapsulation.Nonecomponent SCSS (not a global stylesheet): the plugin repo owns no global stylesheet — globals live in the host repo, whichdevinstall.shdoesn't ship.:has()is supported in all current evergreen browsers (incl. Firefox ESR 128+) — fine for this internal admin app.🤖 Generated with Claude Code