Fix the two focused-interactions e2e failures, and drop the version-bump action - #220
Closed
KohmeiK wants to merge 2 commits into
Closed
Fix the two focused-interactions e2e failures, and drop the version-bump action#220KohmeiK wants to merge 2 commits into
KohmeiK wants to merge 2 commits into
Conversation
The playback controls moved into the left nav rail's Analyze group in #218, so the pan-zoom-slider case timed out looking for a scrubber that only exists while Analyze is open. Open Analyze first, and drag the scrubber down its height — it is a horizontal Material slider rotated 90 degrees, not a horizontal one any more. The mobile horizontal overflow was not from the nav rework (that bar only got shorter): the top toolbar's buttons never wrapped or shrank, so at 390px they ran 851px wide and dragged the whole document sideways, panning the grid and panels off-screen. Contain the overflow in the bar so it scrolls itself. Opening Analyze also surfaced a standing Angular warning: the collapsible subsection's closed state animated `display`, which is not animatable. Drop it and let the box collapse to zero height, clipping its content. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
✅ Deploy Preview for pmksprod ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The action ran `phips28/gh-action-bump-version` on every push to `main`, so each merge pushed a further version-bump commit back to `main` — which is also the production deploy trigger. Version bumps now happen by hand in the PR that ships a release. `package.json`'s version is what the bottom bar shows (via `environment*.ts`), so both agent guides now say to raise it deliberately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
KohmeiK
changed the title
Member
Author
|
Absorbed by #221 |
KohmeiK
added a commit
that referenced
this pull request
Jul 26, 2026
- hull.js (critical, code injection via the points-format parameter): the patched 1.0.10+ was never published to npm — the registry stops at 1.0.6 — so install v1.0.13 from the GitHub tag the advisory points at. The git layout has no dist/ bundle, so the import moves to the package main (typed by @types/hull.js, hence the number[][] casts) and the redundant angular.json global-script entry is dropped. PMKS never passes the vulnerable third parameter, so this was defense in depth. - protobufjs 7.6.5 (runtime, via firebase), postcss, fast-uri, tar, brace-expansion: in-range lockfile updates. - esbuild 0.28.1 and @babel/core 7.29.7 arrive with the Angular 22.0.8 patch set; Angular's exact peer matrix moves the framework and CLI together, which required regenerating the lockfile (all resolutions stay inside the declared ranges). - apexcharts pinned to 5.15.2: the fresh resolve pulled 5.16.0, whose own type declarations do not compile (ApexDrilldownEvent is used but never declared). Left open: @hono/node-server (moderate, Windows path traversal) inside @angular/cli's MCP server — the fix is only in the 2.x major, which @modelcontextprotocol/sdk does not accept yet. Dev-only exposure; it will close with a future Angular CLI release. Verified: 190 unit tests pass, the production build succeeds, and the focused-interactions e2e smoke against the built dist shows only the two known main-branch issues that #220/#221 already fix. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Two independent changes: the e2e fixes below, plus removal of the automated version-bump action (requested separately — see the last section).
Part 1 — e2e failures
Gets
e2e/focused-interactions.mjsback to green: 0 issues across 3 consecutive runs (baseline onmain: 2).1.
Case failed: pan-zoom-slider— script fix#218 moved the playback controls into the left nav rail's Analyze group, so
#sliderdoes not exist while Edit is the open mode andboundingBox()timed out.openAnalyze()helper that checks.playbackControlsbefore clicking, since the rail buttons toggle their own panel (tabClicked) — a blind click would close what the case needs.rotate(90deg)inside.verticalSlider, so the old horizontal drag would have moved it zero even once found. It is now dragged down its height. Verified real, not just non-throwing:beforeValue: "0" → afterValue: "299".button:has-text("play_arrow")→.playbackControls .playButton, which does not break when the icon flips topause.2.
Horizontal overflowon mobile — a real pre-existing app bugNeither of the two suspected causes. It is not a regression from the left-nav rework, and not a stale expectation.
All 851px came from
nav.navBar; no other element exceeded the 390px viewport. #218 only removed buttons from that bar (Undo, Redo, Equations), so pre-#218 it was roughly 170px wider. The bar isdisplay:flex; width:100%withbutton { flex: 0 0 auto; white-space: nowrap }, and the app has essentially no responsive CSS (2 media queries repo-wide, both added by #218).The defect is real, though: document-level overflow means the whole page pans sideways on mobile, dragging the grid and panels off-screen. Fixed by containing the overflow in the bar —
overflow-x: autoplus a hidden scrollbar, since a classic scrollbar would eat half of the 30px strip. Every button stays reachable by swiping the bar. VerifiedscrollWidth851 → 390; desktop rendering unchanged.I deliberately did not restyle the bar to icon-only below a breakpoint — that is a design call. It would fit (7 buttons x ~40px + logo ~= 343px) and every button already carries a tooltip.
3. Baseline flakes, and one that was not flaky
Visible NaN in UIandCase failed: link-context: flaky — zero occurrences in 6 runs. Left alone.openClosetrigger animatingdisplay, fromcollapsible-subseciton. Angular warns and drops the property, so the collapse was relying on something that does not work. Removeddisplay: 'none'from the closed state and added theoverflow: hiddenon.panel-contentit needs in order to hide content at zero height.Reviewer notes
collapsible-subsecitonis a shared BLOCK used by 5 panels (edit, analysis, equation, settings, synthesis), so that change is the widest-reaching one here. I verified collapse/expand visually and by measurement (height: 208 → 0 → 208, no bleed-through of children past the collapsed box).Verification
npm test -- --watch=false— 190/190 passing (26 files)e2e/focused-interactions.mjs— 0 issues, 3 consecutive runse2e/left-nav-modes.mjs— 23/23e2e/force-analysis-panels.mjs— 14/14Related, left out of scope
e2e/deep-interactions.mjshas the same Force analysis panels, left nav rework, and unified unit inputs #218 breakage (Animation slider not visible,Play button click failed). It.catch()es instead of throwing, so it degrades to warnings rather than a hard failure. The sameopenAnalyze+ vertical-drag fix applies.e2e/full-tour.mjsmobile overflow has a different source — 516 vs 390 after this fix: 96px rail + the 420px Analyze panel. Partly worsened by Force analysis panels, left nav rework, and unified unit inputs #218 (rail went ~48px → 96px), but the 420px panel already did not fit at 390px before it.Also worth reconciling separately:
CLAUDE.mdstill says to route UI validation to GPT-5.5 viacodex exec, but.claude/skills/ui-validate/SKILL.mdhas since been rewritten to run Playwright directly. I followed the skill.Part 2 — remove the automated version bump action
Deletes
.github/workflows/bumpversion.yml, which ranphips28/gh-action-bump-versionon every push tomain.Worth knowing:
mainis also the production deploy trigger, so every merge produced a second commit (the bump) pushed straight back tomain. Removing this makes version bumps deliberate.Consequence for reviewers:
package.json'sversionis what the bottom bar displays (throughsrc/environments/environment.ts/environment.prod.ts). It is currently2.0.3and will now stay there until someone raises it by hand — so a release PR needs to include the bump. I updated the branch-rules section of bothCLAUDE.mdandAGENTS.md, which previously documented the action as the mechanism..github/workflows/verification.yml(tests + build on PRs andmain) is untouched.🤖 Generated with Claude Code