fix: overflow of some components - #1016
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughNavigation instrument styles now clip compass-sector host overflow and render SVG elements as blocks in thruster, watch-flat, and watch. Radial instrument guidance documents the clipping behavior, SVG layout behavior, arrow-apex shaving, and geometry input updates. ChangesNavigation instrument rendering
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Evaluated this PR against latest Root causes (reverse-engineered)1. Inline-SVG line-box gap — the three 2. Rotated element-box overflow — the compass-sector Verification against latest
|
…selines
Removes the two accidentally committed root-level screenshots and
regenerates the 8 compass-sector baselines affected by the new
:host{overflow:hidden}: the HDG/COG arrow apexes previously painted
1-2px outside the host box (overflow:visible) and are now clipped at
the edge — imperceptible at normal viewing, verified sharp in the new
baselines. Full radial family re-verified byte-identical on top of
latest develop (incl. #1049): watch, watch-flat, azimuth(+thruster),
compass, heading, gauge-radial, instrument-radial, rudder, speed-gauge,
rot-sector, wind, compass-flat — 155 tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Applied the changes from the evaluation above directly on this branch:
Verification on the updated branch: compass-sector 12/12 stable on re-run; full radial sweep byte-identical — watch 21, watch-flat 2, azimuth(+thruster) 25, compass 9, heading 3, gauge-radial 32, instrument-radial 10, rudder 13, speed-gauge 7, rot-sector 9, wind 18, compass-flat 6 (155 tests). Typecheck clean. From my side this is now ready for review/merge. |
…revisit path
Records why :host{overflow:hidden} exists (rotated overlay element box
leaking arc pixels in wide layouts), the accepted 1-2px HDG/COG arrow
apex shave it introduces, and the designer revisit path (frame headroom
instead of removing the clip) — including the interaction cautions with
the #1021/#1049 label-reserve geometry, in both compass-sector.css and
watch-radial-instruments.instructions.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The storybook can be viewed at https://openbridge-next-storybook--1016-fix-overflow-of-some-c-jy7ojzuw.web.app |
|
The demo can be viewed at https://openbridge-next-demo--1016-fix-overflow-of-some-compon-x19oxcqt.web.app |
…e of truth The merge of develop brought #1118 (docs/agents is now the single source; .github/instructions/* and .cursor/rules/* are generated), so the section PR #1016 added to the generated instructions file would have been wiped by the next agents:sync. Move it into docs/agents/watch-radial-instruments.md and regenerate both adapters. While porting: fix a prettier-mangled bullet ("PADDING = 72 + per-FOV" had turned into a nested list), point the compass-sector.css comment at the docs/agents path instead of the generated file, and add a paragraph recording why the leak needs a cropped frame and why rotating an inner <g> (which would make the svg's own viewport clip sufficient) was not taken: shared-core refactor, family-wide baseline churn, no visual gain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Brought the branch up to date with
Re-verification on top of next.122:
Still ready for review/merge from my side. |
…nts (#1130) * fix(instruments): remove inline-svg line-box gap in remaining components Aligns the last four components rendering an in-flow inline <svg> with the display:block pattern established across the families and extended by #1016: bearing-indicator and compass-indicator get the rule in their (previously empty) stylesheets; gauge-horizontal and gauge-vertical render into light DOM (createRenderRoot returns this), where static styles never attach, so they carry it on the svg's existing inline style binding instead. Four comparison-story baselines compact: stacked in-flow gauges sat ~4px apart on phantom descender space, which was the bug. Single- instance stories, both indicators and the composite consumers (gauge-bar-indicator, setpoint) are byte-identical. Also adds the two prevention checklist lines to docs/agents/watch-radial-instruments.md (element-box rotation needs a host clip; in-flow svgs need display:block) and regenerates the instruction adapters. Part of #1129 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(agents): record the measured verdict on inner-<g> rotation Replaces the speculative churn/perf clause in the host-clipping section with the #1129 measurements: element-level transform updates reuse the display list (~3x less main-thread Paint at 10 Hz than rotating an inner <g>, raster equal), and the swap is only a drop-in on origin-centred viewBoxes — on compass-sector's cropped frame the rotation centres differ and a naive swap displaces the arc. Part of #1129 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary by CodeRabbit
Bug Fixes
Style
Documentation