feat(ar)!: poi consumability overhaul with selection api, grouping events, projection module, and richer detections - #1060
Open
ludvigovrevik wants to merge 27 commits into
Open
feat(ar)!: poi consumability overhaul with selection api, grouping events, projection module, and richer detections#1060ludvigovrevik wants to merge 27 commits into
ludvigovrevik wants to merge 27 commits into
Conversation
…hreshold contract Grouping thresholds previously defaulted to enter=0/exit=8, so targets only grouped when literally touching and ungrouped 8px later - the config every story overrides and the first thing external consumers trip over. Defaults now match the story-proven values (enter 10px, exit 18px, pre 16px, behind 10px), explicit 0px values are respected (the old `|| 0` fallback coerced them), and exit is clamped to at least enter so a misconfigured band cannot oscillate. The class JSDoc now documents all seven layer CSS custom properties and the actual x/y stability contract for animated scenarios. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PoiBase hard-coded its internal x low-pass filter at 16 Hz, which only removes near-frame-rate noise and fights consumers that run their own smoothing. New xFilterCutoffHz property (attribute x-filter-cutoff-hz, default 16) lets consumers trade responsiveness for stability; 0 or negative disables filtering entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r line-length var The family shipped three different line-length defaults: obc-poi 96, the poi-data/vessel/aton variants 192, and an unused --obc-poi-layer-line-length: 120px that nothing consumes. obc-poi now shares the variants' 192 via an exported DEFAULT_LINE_LENGTH_PX constant, and the dead CSS var is removed. BREAKING CHANGE: obc-poi used directly without an explicit `y` now renders a 192px line (was 96px). Set `y` explicitly to keep the old length. The no-op --obc-poi-layer-line-length custom property is gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tribute The camelCase @Property reflected as an all-lowercase `positionvertical` attribute, forcing poi-layer to probe both spellings and making the documented `positionVertical="..."` HTML usage work only by accident of HTML case-folding. The attribute is now the family-standard kebab-case `position-vertical`; the JS property name is unchanged. BREAKING CHANGE: the `positionvertical`/`positionVertical` attribute form is no longer observed; use `position-vertical` (JS property `positionVertical` is unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ption lit labs gen builds import specifiers with the OS path separator; on Windows the backslashes are consumed as string escapes when generated sources are written (\b even becomes a backspace), yielding broken specifiers like "@oicl/openbridge-webcomponents/distarpoi-card...". The damage is unrecoverable post-hoc, so wrappers:post-fix now scans generated sources and aborts with guidance to generate on Linux/macOS/WSL instead of letting a corrupt build land silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng events - obc-poi-layer-stack: Required Setup checklist (controller stack slot, single is-selected layer, background layer marker, min-height) and Framework Notes warning that the stack re-parents slotted POI elements (declarative renderers must not own POI children) - obc-poi-controller: matching Framework Notes; the detections path is the framework-safe route - PoiBase + variants: Coordinate Model section (x center px, bottom-edge anchor, y = downward connector length, built-in x filter) - obc-poi-group: document obc-poi-group-target-released (@fires) and resolve the collapsing/internalSwapping TODOs as orchestration state - obc-poi-button: inExpandedGroup documented as group-applied state - obc-poi-layer: layer-resize only needed standalone, requestGroupingUpdate as the imperative-mutation hook, min-height guidance Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Consumers previously had to copy the magic strings for data-controller-layer="background" from documentation. The attribute name and value are now exported as POI_CONTROLLER_LAYER_ATTR and POI_CONTROLLER_BACKGROUND_LAYER and used internally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The grouped stories snapshot mid-animation, racing the grouping pipeline; the wider default thresholds made the race visible as run-to-run flakes. Await waitForStorySettle like the poi-layer stories do. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Selection state was previously unreachable: no event fired on user
selection, nothing exposed the selected set, and badge numbering was an
emergent side effect of a private counter — consumers resorted to
synthetic .click() calls, click-suppression sets, and poking the
private selectionCounter field.
New API:
- selectedTargets getter
- selectTarget(target, {selectionId?}) / deselectTarget(target) /
clearSelection() — same flow as user clicks; selectionId presets the
stack-managed badge id
- selection-change CustomEvent {selected, added, removed} fired on
every mutation (click, programmatic, bootstrap seeding)
The click handler now shares performSelection() with the programmatic
path. New SelectionHandling story exercises selectTarget + selectionId
+ selection-change end-to-end.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Grouping/overlap state was only observable via the layer's internal
data-* attributes, forcing consumers into MutationObservers on
undocumented internals. The layer now dispatches a grouping-change
CustomEvent {clusters, front, behind, pregrouped} whenever membership
or overlap state actually changes (signature-compared to suppress
per-frame noise).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cover/contain letterbox projection lived only inside obc-poi-controller's private mapDetection, forcing applications that manage their own POI elements to re-implement it (the reference AR app carries a 190-line port). New src/ar/poi-projection module exports computeMediaProjection, projectPoint, projectPointToLayer, and projectBoxSize; the controller now consumes the module, making it the single source of truth. Includes a pattern-b documented story with an interactive letterbox demo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
x updates were filtered inside PoiBase but y passed straight through, so consumers feeding live coordinates had to bolt their own smoothing onto y (the reference AR app runs a hand-rolled critically-damped spring for exactly this). y now gets the same 1-pole low-pass as x, with a matching yFilterCutoffHz property (attribute y-filter-cutoff-hz, default 16 Hz, 0 or negative disables). The filter initializes at the first value, so static usage renders identically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tections path
The detections API only produced bare obc-poi-data targets, so any
application needing vessel/aton variants, icons, or data rows had to
abandon the framework-safe controller path and manage POI elements
imperatively. Detections now accept variant ('data' | 'vessel' |
'aton'), icon (child element tag), state, and data; the controller
recreates targets on variant change and swaps icon children in place.
Controller-level x/y-filter-cutoff-hz forward to every owned target.
New DetectionVariants story renders all three variants from a single
detections array.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
close-click (dispatched by the embedded header, re-targeted through the card's shadow root) had no @fires tag, so generated framework wrappers omitted the event. The card body also exposed no shadow parts, forcing consumers to inject adoptedStyleSheets for styling as small as a drop-shadow. Adds @fires close-click and wrapper/content-box/header/ content parts with @csspart docs; rendering verified pixel-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documents why the POI family's light-DOM mutations (auto-group creation, selection FLIP re-parenting, header relocation) break declarative renderers, evaluates shadow mirrors vs manual slot assignment vs controller-owned generation, and recommends the staged path: the now-shipped detections API as the framework-safe route, and manual slot assignment as the next-major end-state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ludvigovrevik
requested review from
jon-daeh,
tibnor and
ulrik-jo
as code owners
July 24, 2026 00:32
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds reusable media projection and POI coordinate filtering, variant-aware controller targets, slot-based grouping, selection projection, header forwarding, CSS parts, public events, and Storybook validation scenarios. ChangesPOI projection and coordinate filtering
Controller target variants
Grouping and selection
Component contracts and validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Detection
participant PoiController
participant Projection
participant PoiTarget
Detection->>PoiController: provide variant and media coordinates
PoiController->>Projection: compute projection and project point
Projection-->>PoiController: rendered position
PoiController->>PoiTarget: create or update POI target
PoiController->>PoiTarget: apply state, data, icon, and filter cutoffs
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
The demo can be viewed at https://openbridge-next-demo--1060-feat-ar-poi-consumability-o-om8dpeaw.web.app |
|
The storybook can be viewed at https://openbridge-next-storybook--1060-feat-ar-poi-consumabil-cojts08r.web.app |
This reverts commit 7da15df.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/proposals/poi-dom-ownership.md (1)
94-101: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd DOM-ownership assertions to the regression plan.
Visual snapshots will not catch the original failure mode. Add tests verifying header node identity, parentage, listeners/state, and framework re-rendering while selection and grouping are active.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/proposals/poi-dom-ownership.md` around lines 94 - 101, Add DOM-ownership assertions to the “Test plan” in poi-dom-ownership.md, covering header node identity, correct parentage, preserved listeners and state, and continued framework re-rendering while selection and grouping are active. Retain the existing visual, regression-story, and reference-app acceptance coverage.packages/openbridge-webcomponents/src/ar/poi-controller/poi-controller.ts (1)
469-483: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist the projection computation out of the per-detection loop.
mapDetectionrecomputescomputeMediaProjectionfrom scratch for every detection, even thoughmediaWidth/mediaHeight/renderWidth/renderHeight/fitare constant for the wholesyncTargetsToCustomStackpass. This repeats the same work N times on a RAF-scheduled hot path.♻️ Compute the projection once per sync
private mapDetection( - det: PoiDetection - ): {x: number; y: number; scale: number} | null { - const projection = computeMediaProjection({ - mediaWidth: this.mediaWidth, - mediaHeight: this.mediaHeight, - renderWidth: this.renderWidth, - renderHeight: this.renderHeight, - fit: this.fit === PoiFitMode.Cover ? MediaFit.Cover : MediaFit.Contain, - }); - if (!projection) return null; - - const point = projectPoint(projection, det.x, det.y); + det: PoiDetection, + projection: MediaProjection + ): {x: number; y: number; scale: number} { + const point = projectPoint(projection, det.x, det.y); return {x: point.x, y: point.y, scale: projection.scale}; }Then in
syncTargetsToCustomStack, computeprojectiononce (bail out viaclearTargets()when it'snull) beforeactive.forEach, and pass it intothis.mapDetection(det, projection).Also applies to: 507-512
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/openbridge-webcomponents/src/ar/poi-controller/poi-controller.ts` around lines 469 - 483, Hoist computeMediaProjection out of the per-detection path: update mapDetection to accept a precomputed projection, and in syncTargetsToCustomStack compute it once before active.forEach, calling clearTargets() and returning when it is null. Pass the shared projection to mapDetection for each detection while preserving the existing point mapping and scale behavior.packages/openbridge-webcomponents/src/ar/poi-layer-stack/poi-layer-stack.stories.ts (1)
428-481: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope DOM queries instead of using global
document+ ad-hoc types.This story queries the whole
documentby ID (#selection-handling-stack,#selection-readout, firstobc-poi-vessel) rather than scoping to the story instance, which risks cross-story collisions in composed/portable test runs. It also castsstackto a synthetic inline shape (target: Element) instead of the realObcPoiLayerStack/Poi-typedselectTarget. Other stories in this cohort (e.g.poi-group.stories.ts) usecreateRef/ref()for scoped access — consider following that convention here, and destructurecanvasElementinplayfor the querySelector calls.♻️ Scope queries and use the real type
play: async ({canvasElement}) => { await waitForStorySettle({drainTransitions: true}); - const stack = document.querySelector('`#selection-handling-stack`') as { - selectTarget?: ( - target: Element, - options?: {selectionId?: string} - ) => boolean; - } | null; - const target = document.querySelector('obc-poi-vessel'); + const stack = canvasElement.querySelector( + 'obc-poi-layer-stack' + ) as ObcPoiLayerStack | null; + const target = canvasElement.querySelector('obc-poi-vessel'); if (stack?.selectTarget && target) { stack.selectTarget(target, {selectionId: '7'}); } await waitForStorySettle({drainTransitions: true}); },For the render-side readout update, consider replacing the
id="selection-readout"+document.querySelectorpair with acreateRef/ref()binding, matching the pattern used inpoi-group.stories.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/openbridge-webcomponents/src/ar/poi-layer-stack/poi-layer-stack.stories.ts` around lines 428 - 481, Scope all DOM lookups in SelectionHandling to the story’s canvasElement, using the play function’s canvasElement for stack and vessel queries instead of global document queries. Replace the synthetic stack cast with the actual ObcPoiLayerStack type and its real selectTarget signature, and use createRef/ref() for the selection readout update rather than an ID-based document query.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/proposals/poi-dom-ownership.md`:
- Around line 81-92: Clarify Migration sketch (Option B) by specifying how
legacy layer-nested targets are adapted without preserving the incompatible
light-DOM re-parenting mutation: either document it as a deprecated,
warning-emitting mutation-based exception or define a non-mutating adapter, and
state the chosen behavior alongside the compatibility pass.
In `@packages/openbridge-webcomponents/fix-generated.cjs`:
- Line 22: Update the source-pattern check in fix-generated.cjs to detect
materialized whitespace control characters after the dist prefix, including
newline, tab, carriage return, and form feed, while still allowing valid
dist/... imports. Add regression fixtures covering each corrupted specifier and
the valid import case.
In `@packages/openbridge-webcomponents/src/ar/poi-controller/poi-controller.ts`:
- Around line 570-576: Update the detection synchronization logic around
target.state and target.data to reset each property to its default value when
the corresponding det field is undefined, while preserving reported values when
present. Ensure stale state and data are cleared on subsequent frames that omit
them, consistently with the existing reset behavior for other optional detection
fields.
---
Nitpick comments:
In `@docs/proposals/poi-dom-ownership.md`:
- Around line 94-101: Add DOM-ownership assertions to the “Test plan” in
poi-dom-ownership.md, covering header node identity, correct parentage,
preserved listeners and state, and continued framework re-rendering while
selection and grouping are active. Retain the existing visual, regression-story,
and reference-app acceptance coverage.
In `@packages/openbridge-webcomponents/src/ar/poi-controller/poi-controller.ts`:
- Around line 469-483: Hoist computeMediaProjection out of the per-detection
path: update mapDetection to accept a precomputed projection, and in
syncTargetsToCustomStack compute it once before active.forEach, calling
clearTargets() and returning when it is null. Pass the shared projection to
mapDetection for each detection while preserving the existing point mapping and
scale behavior.
In
`@packages/openbridge-webcomponents/src/ar/poi-layer-stack/poi-layer-stack.stories.ts`:
- Around line 428-481: Scope all DOM lookups in SelectionHandling to the story’s
canvasElement, using the play function’s canvasElement for stack and vessel
queries instead of global document queries. Replace the synthetic stack cast
with the actual ObcPoiLayerStack type and its real selectTarget signature, and
use createRef/ref() for the selection readout update rather than an ID-based
document query.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01a1ce3f-d277-478c-8dd4-f81d794d7c59
📒 Files selected for processing (19)
docs/proposals/poi-dom-ownership.mdpackages/openbridge-webcomponents/fix-generated.cjspackages/openbridge-webcomponents/src/ar/poi-button/poi-button.tspackages/openbridge-webcomponents/src/ar/poi-card/poi-card.tspackages/openbridge-webcomponents/src/ar/poi-controller/poi-controller.stories.tspackages/openbridge-webcomponents/src/ar/poi-controller/poi-controller.tspackages/openbridge-webcomponents/src/ar/poi-group/poi-group.stories.tspackages/openbridge-webcomponents/src/ar/poi-group/poi-group.tspackages/openbridge-webcomponents/src/ar/poi-layer-stack/poi-layer-stack.stories.tspackages/openbridge-webcomponents/src/ar/poi-layer-stack/poi-layer-stack.tspackages/openbridge-webcomponents/src/ar/poi-layer/poi-layer.csspackages/openbridge-webcomponents/src/ar/poi-layer/poi-layer.tspackages/openbridge-webcomponents/src/ar/poi-projection/poi-projection.stories.tspackages/openbridge-webcomponents/src/ar/poi-projection/poi-projection.tspackages/openbridge-webcomponents/src/ar/poi/poi-aton.tspackages/openbridge-webcomponents/src/ar/poi/poi-base.tspackages/openbridge-webcomponents/src/ar/poi/poi-data.tspackages/openbridge-webcomponents/src/ar/poi/poi-vessel.tspackages/openbridge-webcomponents/src/ar/poi/poi.ts
| visit(full); | ||
| } else if (entry.name.endsWith('.ts') || entry.name.endsWith('.js')) { | ||
| const source = fs.readFileSync(full, 'utf8'); | ||
| if (/@oicl\/openbridge-webcomponents\/dist[^/'"\s]/.test(source)) { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Detect whitespace control-character corruption too.
Line 22 excludes \s, so a generated specifier containing a materialized \n, \t, \r, or \f after dist can evade this guard and leave a broken wrapper import in the build output. Add a regression fixture for these cases while continuing to allow valid dist/... imports.
Suggested fix
- if (/@oicl\/openbridge-webcomponents\/dist[^/'"\s]/.test(source)) {
+ if (
+ /@oicl\/openbridge-webcomponents\/dist(?:[^/'"\s]|[\u0000-\u001f])/.test(
+ source
+ )
+ ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (/@oicl\/openbridge-webcomponents\/dist[^/'"\s]/.test(source)) { | |
| if ( | |
| /@oicl\/openbridge-webcomponents\/dist(?:[^/'"\s]|[\u0000-\u001f])/.test( | |
| source | |
| ) | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openbridge-webcomponents/fix-generated.cjs` at line 22, Update the
source-pattern check in fix-generated.cjs to detect materialized whitespace
control characters after the dist prefix, including newline, tab, carriage
return, and form feed, while still allowing valid dist/... imports. Add
regression fixtures covering each corrupted specifier and the valid import case.
…or corruption" This reverts commit 6f45815.
…and new stories Regenerated via the CI-matching Docker image for the stories affected by the new grouping thresholds (grouped poi-group variants, animated stack selection) plus baselines for the new SelectionHandling, DetectionVariants, and poi-projection stories. Verified stable with a second no-update run; the flaky mid-animation crossing-mode capture is left at its previous CI-authoritative baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ludvigovrevik
changed the title
state/data assignments were guarded on presence, so a detection that stopped reporting them kept the stale alert state or data table indefinitely. Reset to defaults each sync, matching how box dimensions and icon children already behave. (CodeRabbit review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mapDetection rebuilt the projection for every detection although the inputs are constant across a sync; hoist it out of the rAF-hot loop. (CodeRabbit review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
obc-poi and the PoiBase variants now render a forwarding <slot name=header slot=header> into the inner poi button, so header elements slotted by the consumer stay in the consumer's light DOM and reach the button through slot assignment. The MutationObserver-based relocation that physically appended consumer nodes into shadow roots is removed. obc-poi-button resolves forwarded slots when detecting header content, re-renders on bubbled slotchange, and is now the single owner of slotted header state. obc-poi's button-slot handler ignores slotchange events bubbling from nested forwarding slots, which previously made it treat forwarded content as a custom button and spray button properties onto it. Adds a browser regression spec pinning header DOM ownership, the slot chain, and framework-style node replacement. BREAKING CHANGE: header elements slotted into POI components are no longer moved into the component's shadow DOM; they remain consumer-owned light DOM children. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
obc-poi-layer-stack no longer moves selected targets between layer elements. Each tracked target keeps an immutable home layer (its DOM parent) plus a logical origin layer; selection is rendered by projecting the button into the selected layer (--obc-poi-button-projection-y) and, for targets authored inside the selected layer, the pointer into the inferred origin layer (--obc-poi-target-projection-y). The existing Web-Animations FLIP animates the same measured jump, so geometry and motion are pixel-identical to the physical move. Deselected bootstrap targets that render outside their DOM layer are tracked as displaced and their projections refresh on every placement pass, so layer resizes cannot leave stale offsets. The write-only previousAnimatePosition record field is dropped. BREAKING CHANGE: selected POI elements are no longer re-parented into the selected layer; they remain children of the layer the consumer placed them in, with data-stack-selected set while selected. Consumers that located selected targets by querying the selected layer's children must use the stack selection API instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…assignment obc-poi-layer now uses a slotAssignment: manual shadow root. Auto-created obc-poi-group elements live in the layer shadow root and receive their member targets through slot assignment into a nested member slot; the targets themselves never leave the consumer light DOM. Grouping, ungrouping, joining an expanded group, and front-child paint ordering all happen by re-assigning slots instead of re-parenting nodes. obc-poi-group gains an assigned-membership mode: membership checks, front-child reordering, and releaseTarget() detect slot-assigned members and re-assign instead of moving DOM, delegating main-slot restoration to the host layer. Consumer-managed light-DOM groups keep the existing behavior. The layer exposes a public autoGroups accessor as the supported way to reach auto-group chrome (the layer stories now use it together with the grouping-change event instead of querying light DOM). Grouped-member styling moves from poi-group.css ::slotted rules into poi-layer.css under the member slot; main-slot rules are scoped so grouped and ungrouped targets keep their exact previous visuals. Four baselines are regenerated: grouped-member connector lines lost a GPU-compositing artifact the old baselines had captured (re-parenting churn kept will-change active at snapshot time), and now rasterize identically to ungrouped targets. DOM, geometry, and computed styles were verified identical before accepting the new renders. BREAKING CHANGE: auto-created obc-poi-group elements no longer appear in the consumer light DOM and targets are never re-parented into them. Consumers that queried the layer children for [data-auto-group] must use the layer autoGroups accessor (typically after a grouping-change event). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Framework Notes warned that slotted POI children break framework reconciliation because of selection re-parenting and light-DOM group injection. Both mechanisms are gone: selection projects via CSS custom properties and auto-groups are shadow-internal slot-assigned chrome, so the notes now state that framework-owned POI children are fully supported on both the detections path and the slotted path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
POI targets carry data-x-moving (a will-change compositing hint) for 120ms after position updates, and whether that window is still open at capture time changes how connector lines and card text rasterize. Several story snapshots captured whichever state machine load produced, which made them flip between two renders. waitForStorySettle now waits for the movement hint to expire (bounded at 2s so continuously animating stories behave as before), the layer-stack and controller stories reuse the shared helper instead of local copies, and the poi-layer stories that captured immediately after render gained the standard settle play. Six baselines are regenerated in the steady state; two consecutive full AR sweeps reproduce them exactly under normal load. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every grouping pass treated already-exiting (dissolved) auto-groups as reusable, found their member list empty, re-disbanded them, and reset their removal timer. Under continuous position churn — live video detections update every frame — the timer never fired and dissolved group chrome accumulated without bound (observed: 165 stale shadow groups after a couple of minutes of playback). Exiting groups are now excluded from the reuse set, so a dissolved group is condemned exactly once and its removal delay runs undisturbed. The regression test asserts the group disappears while churn is still running; it was verified to fail against the previous behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The story rasterizes in one of two compositing states and CI consistently produces the other one from this machine's docker capture. Baseline is the actual-render third of the CI diff artifact, per the repo playbook for renderer-divergent snapshots. 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.
Why
A consumer report (POI grouping unusable with moving x/y) triggered a two-sided audit: the component API surface, and every workaround our own AR platform (ar.bridgable.ai) had built around it. The findings: unusable grouping defaults, selection reachable only through private fields and synthetic clicks, grouping state exposed only via undocumented data-* attributes, sealed projection math, and ~970 lines of app-side compensation. This PR fixes the API side; the reference app has been migrated on top of it as validation.
What
Fixes
0pxrespected; exit clamped ≥ enter. All seven layer CSS vars + the x/y stability contract documented.positionVerticalexposed as standardposition-verticalattribute (BREAKING: old lowercase attribute form no longer observed).DEFAULT_LINE_LENGTH_PX(BREAKING: bareobc-poidefault was 96); dead--obc-poi-layer-line-lengthremoved.Features
xFilterCutoffHz/yFilterCutoffHzon all POI targets (built-in x and new y low-pass, 0 disables); forwarded from the controller.obc-poi-layer-stack:selectTarget(target, {selectionId}),deselectTarget,clearSelection,selectedTargets, and aselection-changeevent (bubbles/composed,{selected, added, removed}) fired for user clicks and programmatic changes alike.grouping-changeevent onobc-poi-layer({clusters, front, behind, pregrouped}, signature-deduped) — no more MutationObservers on internal attributes.poi-projectionpure-function module (computeMediaProjection,projectPoint,projectPointToLayer,projectBoxSize); controller refactored onto it.variant(data/vessel/aton),icon,state,data— the framework-safe path now covers real applications.POI_CONTROLLER_LAYER_ATTR/POI_CONTROLLER_BACKGROUND_LAYERconstants.Docs
PoiBase;@firesforobc-poi-group-target-releasedandclose-click; CSS parts onobc-poi-card.Chores
fix-generated.cjsnow fails fast when Windows path-separator corruption would write broken wrapper imports.Validation
CI note
Expect visual baseline diffs on first run: the y-default unification (bare
obc-poistories) plus the new stories need fresh Linux baselines — I'll pull them from the Playwright artifact and push, per the repo workflow. Wrapper packages regenerate on CI (local generation is Windows-broken; see the new guard).🤖 Generated with Claude Code
Summary by CodeRabbit
obc-poi-controllercan render POI variants per detection (data/vessel/aton) with optional icon and state.obc-poi-layer-stackselection API is enhanced (selected target tracking, select/deselect/clear) and emitsselection-change.position-verticalusage and newobc-poi-group-target-released/obc-poi-layergrouping-changeevent guidance.obc-poi-cardnow exposes additional::part(...)hooks and documentsclose-click.