fix(ui): the inspector dropped labels for inset pin columns, and flickered
Three things the dialog got wrong in use: 1. Missing pins. Classification was nearest-edge with an "interior" bucket, and interior pins printed no label - only a dot with a tooltip. The ReSpeaker Lite showed it: its XIAO socket is two INSET columns (x=50.5 and 126.5 on a 178-wide body) plus two breakout columns, so most of its pins were called interior and the part looked like it was missing them. The classifier now finds the LINES first - pins sharing an x are a column, pins sharing a y a row - assigns each line to the side of the body it sits on however far inset it is, and joins label to dot with a leader. Only a lone pad falls back to nearest-edge. Every pin gets a printed label; that is now the contract, covered by a ReSpeaker test with its real table. 2. The panel was cut off on the Datasheet tab until some unrelated event re-rendered it. The datasheet loads asynchronously, so the one-shot measure placed the dialog while it was still short and it then grew off screen. A ResizeObserver repositions on any size change, and the tab body caps its own height so long markdown scrolls inside the tab. 3. Flicker when the pointer left and re-entered. The preview effect depended on componentProperties and pinInfo by identity - both are rebuilt on every parent render (pinInfo is a getter) - so the element was torn down and remounted constantly. It now depends on stable digests of them.
This commit is contained in:
David Montero Crespo
parent
b02aab9a47
commit
cbc74d8409
|
|
@ -148,20 +148,50 @@ describe('layoutInspectorPins — a crowded header fits its edge', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('layoutInspectorPins — interior pins', () => {
|
||||
it('detects a pin far from every edge and anchors its label on the dot', () => {
|
||||
const r = layoutInspectorPins(
|
||||
[
|
||||
{ name: 'MID', x: 50, y: 50 },
|
||||
{ name: 'L', x: 2, y: 50 },
|
||||
],
|
||||
{ width: 100, height: 100 },
|
||||
);
|
||||
const mid = r.pins.find((p) => p.name === 'MID')!;
|
||||
expect(mid.edge).toBe('interior');
|
||||
expect(mid.labelX).toBe(mid.dotX);
|
||||
expect(mid.labelY).toBe(mid.dotY);
|
||||
expect(r.pins.find((p) => p.name === 'L')!.edge).toBe('left');
|
||||
describe('layoutInspectorPins — inset columns (the ReSpeaker Lite case)', () => {
|
||||
// pro/.../SeeedElements.ts ReSpeakerLiteElement.pinInfo, body 178x437.
|
||||
// The XIAO socket is TWO INSET COLUMNS (x=50.5 / 126.5 on a 178-wide body)
|
||||
// plus two breakout pad columns at x=10. A nearest-edge rule called most of
|
||||
// these "interior" and printed no label, so the part looked like it was
|
||||
// missing pins. Every pin must get a label.
|
||||
const RESPEAKER: InspectorPinInput[] = [
|
||||
...['D0', 'D1', 'D2', 'D3', 'D4', 'D5', 'D6'].map((name, i) => ({
|
||||
name,
|
||||
x: 50.5,
|
||||
y: [43, 55, 68, 81, 93, 106, 119][i],
|
||||
})),
|
||||
...['D7', 'D8', 'D9', 'D10', '3V3', 'GND', '5V'].map((name, i) => ({
|
||||
name,
|
||||
x: 126.5,
|
||||
y: [43, 55, 68, 81, 93, 106, 119][i],
|
||||
})),
|
||||
...['BTN1', 'BTN2', 'BTN3'].map((name, i) => ({ name, x: 10, y: 214 + i * 15 })),
|
||||
...['IO1', 'IO2', 'IO3', 'IO4'].map((name, i) => ({ name, x: 10, y: 289 + i * 13 })),
|
||||
];
|
||||
|
||||
const r = layoutInspectorPins(RESPEAKER, { width: 178, height: 437 });
|
||||
|
||||
it('labels every single pin — none silently dropped', () => {
|
||||
expect(r.pins).toHaveLength(RESPEAKER.length);
|
||||
const named = new Set(r.pins.map((p) => p.name));
|
||||
for (const p of RESPEAKER) expect(named.has(p.name)).toBe(true);
|
||||
});
|
||||
|
||||
it('keeps each inset column on the side of the body it sits on', () => {
|
||||
const sideOf = (n: string) => r.pins.find((p) => p.name === n)!.edge;
|
||||
for (const n of ['D0', 'D3', 'D6']) expect(sideOf(n)).toBe('left');
|
||||
for (const n of ['D7', '3V3', '5V']) expect(sideOf(n)).toBe('right');
|
||||
for (const n of ['BTN1', 'IO4']) expect(sideOf(n)).toBe('left');
|
||||
});
|
||||
|
||||
it('draws a leader from every inset dot to its gutter label', () => {
|
||||
for (const n of ['D0', 'D7', '3V3']) {
|
||||
expect(r.pins.find((p) => p.name === n)!.needsLeader).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('never puts a column end-pin in the top or bottom gutter', () => {
|
||||
expect(r.pins.some((p) => p.edge === 'top' || p.edge === 'bottom')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -168,6 +168,15 @@
|
|||
display: block;
|
||||
}
|
||||
|
||||
/* Stand-in for the part's body while the art is hidden (pin mode). */
|
||||
.pid-body-outline {
|
||||
position: absolute;
|
||||
border: 1px dashed #5a5a5a;
|
||||
border-radius: 4px;
|
||||
background: rgba(255, 255, 255, 0.02);
|
||||
pointer-events: none;
|
||||
}
|
||||
|
||||
.pid-preview-missing {
|
||||
color: #666;
|
||||
font-size: 22px;
|
||||
|
|
@ -352,7 +361,9 @@
|
|||
overscroll-behavior: contain;
|
||||
-webkit-overflow-scrolling: touch;
|
||||
min-height: 120px;
|
||||
max-height: min(52vh, 420px);
|
||||
/* Cap the tab, not the dialog: a long datasheet scrolls INSIDE the tab so
|
||||
the dialog keeps a stable height and never runs off the screen. */
|
||||
max-height: min(46vh, 360px);
|
||||
}
|
||||
|
||||
/* The datasheet reuses the picker card's .cip-doc styles as-is. */
|
||||
|
|
|
|||
|
|
@ -76,8 +76,14 @@ interface PartInspectorDialogProps {
|
|||
wireInProgress?: boolean;
|
||||
}
|
||||
|
||||
const PREVIEW_ART = 150; // collapsed preview box
|
||||
const PINS_ART = 240; // art box when the pin view is on
|
||||
const PREVIEW_ART = 150; // collapsed preview box (the part's own art)
|
||||
/**
|
||||
* Pin-map box. Smaller than the collapsed preview on purpose: in this mode the
|
||||
* part's art is HIDDEN (it ate the dialog for no information) and only the pin
|
||||
* dots, their labels and a faint outline of the body are drawn, so the spatial
|
||||
* arrangement still reads while the panel stays compact.
|
||||
*/
|
||||
const PINS_ART = 190;
|
||||
|
||||
export const PartInspectorDialog: React.FC<PartInspectorDialogProps> = ({
|
||||
componentId,
|
||||
|
|
@ -147,6 +153,17 @@ export const PartInspectorDialog: React.FC<PartInspectorDialogProps> = ({
|
|||
return () => window.removeEventListener('keydown', onCaptureKey, true);
|
||||
}, [capturingKey, componentId, onPropertyChange]);
|
||||
|
||||
// The preview effect must NOT re-run on every parent render, or the element
|
||||
// is torn down and remounted and the dialog visibly flickers whenever the
|
||||
// canvas re-renders (moving the pointer in and out is enough). Both of the
|
||||
// inputs it cares about arrive as fresh objects each render — pinInfo is a
|
||||
// getter that builds a new array, and componentProperties a new object — so
|
||||
// depend on stable string digests of them instead of their identities.
|
||||
const propsKey = Object.entries(componentProperties)
|
||||
.map(([k, v]) => `${k}=${typeof v === 'string' && v.length > 32 ? v.length : String(v)}`)
|
||||
.join('|');
|
||||
const pinsKey = pinInfo.map((p) => `${p.name}:${p.x},${p.y}`).join('|');
|
||||
|
||||
// ── Live preview: mount the real element, measure, lay out pins ──────────
|
||||
useEffect(() => {
|
||||
const host = previewHostRef.current;
|
||||
|
|
@ -218,26 +235,45 @@ export const PartInspectorDialog: React.FC<PartInspectorDialogProps> = ({
|
|||
el.removeEventListener('pininfo-change', onPinsChange);
|
||||
host.textContent = '';
|
||||
};
|
||||
// componentProperties is intentionally in the deps: a color change while
|
||||
// the dialog is open should repaint the preview.
|
||||
}, [componentMetadata, componentProperties, pinInfo, showPins]);
|
||||
// propsKey/pinsKey stand in for componentProperties/pinInfo: a real change
|
||||
// (a colour edit, an LED flip) changes the digest and repaints; a bare
|
||||
// re-render does not.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [componentMetadata, propsKey, pinsKey, showPins]);
|
||||
|
||||
// ── Position: viewport coords, clamped, portaled to body ─────────────────
|
||||
// Re-run on every SIZE change, not just on the inputs we can name. The
|
||||
// datasheet arrives asynchronously (loadDoc) and the markdown can be long,
|
||||
// so a one-shot measure placed the dialog while it was still short and it
|
||||
// then grew off the bottom of the screen — the user only saw it corrected
|
||||
// when some other event happened to re-render and re-measure it.
|
||||
useLayoutEffect(() => {
|
||||
const el = dialogRef.current;
|
||||
if (!el) return;
|
||||
const margin = 12;
|
||||
const w = el.offsetWidth;
|
||||
const h = el.offsetHeight;
|
||||
const vw = window.innerWidth;
|
||||
const vh = window.innerHeight;
|
||||
let x = position.x + 16;
|
||||
let y = position.y;
|
||||
if (x + w > vw - margin) x = Math.max(margin, position.x - w - 16);
|
||||
x = Math.max(margin, Math.min(x, vw - w - margin));
|
||||
y = Math.max(margin, Math.min(y, vh - h - margin));
|
||||
setDialogPos({ x, y });
|
||||
}, [position, layout, tab, showPins]);
|
||||
const place = () => {
|
||||
const margin = 12;
|
||||
const w = el.offsetWidth;
|
||||
const h = el.offsetHeight;
|
||||
const vw = window.innerWidth;
|
||||
const vh = window.innerHeight;
|
||||
let x = position.x + 16;
|
||||
let y = position.y;
|
||||
if (x + w > vw - margin) x = Math.max(margin, position.x - w - 16);
|
||||
x = Math.max(margin, Math.min(x, vw - w - margin));
|
||||
y = Math.max(margin, Math.min(y, vh - h - margin));
|
||||
setDialogPos((prev) =>
|
||||
prev && Math.abs(prev.x - x) < 0.5 && Math.abs(prev.y - y) < 0.5 ? prev : { x, y },
|
||||
);
|
||||
};
|
||||
place();
|
||||
const ro = new ResizeObserver(place);
|
||||
ro.observe(el);
|
||||
window.addEventListener('resize', place);
|
||||
return () => {
|
||||
ro.disconnect();
|
||||
window.removeEventListener('resize', place);
|
||||
};
|
||||
}, [position]);
|
||||
|
||||
// ── Close behaviours (unchanged semantics) ───────────────────────────────
|
||||
useEffect(() => {
|
||||
|
|
@ -376,18 +412,16 @@ export const PartInspectorDialog: React.FC<PartInspectorDialogProps> = ({
|
|||
onClick={clickable ? activate : undefined}
|
||||
disabled={!clickable}
|
||||
/>
|
||||
{p.edge !== 'interior' && (
|
||||
<button
|
||||
type="button"
|
||||
className={`pid-pin-label pid-pin-label--${p.edge}`}
|
||||
style={{ left: ox + p.labelX, top: oy + p.labelY }}
|
||||
title={pinTitle(p.name, pinInfo.find((q) => q.name === p.name)?.description)}
|
||||
onClick={clickable ? activate : undefined}
|
||||
disabled={!clickable}
|
||||
>
|
||||
{p.name}
|
||||
</button>
|
||||
)}
|
||||
<button
|
||||
type="button"
|
||||
className={`pid-pin-label pid-pin-label--${p.edge}`}
|
||||
style={{ left: ox + p.labelX, top: oy + p.labelY }}
|
||||
title={pinTitle(p.name, pinInfo.find((q) => q.name === p.name)?.description)}
|
||||
onClick={clickable ? activate : undefined}
|
||||
disabled={!clickable}
|
||||
>
|
||||
{p.name}
|
||||
</button>
|
||||
</React.Fragment>
|
||||
);
|
||||
})}
|
||||
|
|
|
|||
|
|
@ -151,25 +151,83 @@ export function layoutInspectorPins(
|
|||
const W = w * scale;
|
||||
const H = h * scale;
|
||||
|
||||
// Classify each pin by its nearest edge on the scaled art.
|
||||
const fW = interiorFrac * W;
|
||||
const fH = interiorFrac * H;
|
||||
const classified = pins.map((pin) => {
|
||||
const x = pin.x * scale;
|
||||
const y = pin.y * scale;
|
||||
const dLeft = x;
|
||||
const dRight = W - x;
|
||||
const dTop = y;
|
||||
const dBottom = H - y;
|
||||
let edge: PinEdge;
|
||||
if (dLeft > fW && dRight > fW && dTop > fH && dBottom > fH) {
|
||||
edge = 'interior';
|
||||
} else {
|
||||
const min = Math.min(dLeft, dRight, dTop, dBottom);
|
||||
edge = min === dLeft ? 'left' : min === dRight ? 'right' : min === dTop ? 'top' : 'bottom';
|
||||
// Classify each pin.
|
||||
//
|
||||
// Nearest-edge alone is wrong for real parts, and got caught by the
|
||||
// ReSpeaker Lite: its XIAO socket is two INSET columns (x=50.5 and 126.5 on
|
||||
// a 178-wide body) plus two rows of breakout pads. Every one of those sat
|
||||
// far enough from all four edges to be called "interior", which printed no
|
||||
// label — the pins were on screen as dots and looked missing. And a plain
|
||||
// nearest-edge rule scatters a column's end pins to the top/bottom gutter.
|
||||
//
|
||||
// So: find the LINES first. Pins sharing an x form a vertical column, pins
|
||||
// sharing a y form a horizontal row; a column belongs to the side of the
|
||||
// body it sits on, however far inset it is, and a leader line joins its
|
||||
// labels to the dots. Only a pin that belongs to no line falls back to its
|
||||
// nearest edge. Every pin gets a printed label — that is the contract.
|
||||
const TOL = 6 * scale; // pads within this are the same column/row
|
||||
const cluster = (vals: number[]): number[][] => {
|
||||
const idx = vals.map((v, i) => [v, i] as const).sort((a, b) => a[0] - b[0]);
|
||||
const groups: number[][] = [];
|
||||
let cur: number[] = [];
|
||||
let last = NaN;
|
||||
for (const [v, i] of idx) {
|
||||
if (cur.length && Math.abs(v - last) > TOL) {
|
||||
groups.push(cur);
|
||||
cur = [];
|
||||
}
|
||||
cur.push(i);
|
||||
last = v;
|
||||
}
|
||||
if (cur.length) groups.push(cur);
|
||||
return groups;
|
||||
};
|
||||
|
||||
const xs = pins.map((p) => p.x * scale);
|
||||
const ys = pins.map((p) => p.y * scale);
|
||||
const side = new Array<PinEdge | null>(pins.length).fill(null);
|
||||
|
||||
// Vertical columns win first: they are the dominant shape on breakout
|
||||
// boards, shields and every DIP part.
|
||||
for (const g of cluster(xs)) {
|
||||
if (g.length < 2) continue;
|
||||
const spanY = Math.max(...g.map((i) => ys[i])) - Math.min(...g.map((i) => ys[i]));
|
||||
if (spanY < TOL) continue; // that is a row, not a column
|
||||
const cx = g.reduce((s, i) => s + xs[i], 0) / g.length;
|
||||
const e: PinEdge = cx <= W / 2 ? 'left' : 'right';
|
||||
for (const i of g) side[i] = e;
|
||||
}
|
||||
// Then horizontal rows, for whatever is still unassigned (headers).
|
||||
for (const g of cluster(ys)) {
|
||||
const free = g.filter((i) => side[i] === null);
|
||||
if (free.length < 2) continue;
|
||||
const spanX = Math.max(...free.map((i) => xs[i])) - Math.min(...free.map((i) => xs[i]));
|
||||
if (spanX < TOL) continue;
|
||||
const cy = free.reduce((s, i) => s + ys[i], 0) / free.length;
|
||||
const e: PinEdge = cy <= H / 2 ? 'top' : 'bottom';
|
||||
for (const i of free) side[i] = e;
|
||||
}
|
||||
|
||||
const classified = pins.map((pin, i) => {
|
||||
const x = xs[i];
|
||||
const y = ys[i];
|
||||
let edge = side[i];
|
||||
if (!edge) {
|
||||
// A lone pad: nearest edge.
|
||||
const d = [
|
||||
['left', x],
|
||||
['right', W - x],
|
||||
['top', y],
|
||||
['bottom', H - y],
|
||||
] as Array<[PinEdge, number]>;
|
||||
d.sort((a, b) => a[1] - b[1]);
|
||||
edge = d[0][0];
|
||||
}
|
||||
return { pin, x, y, edge };
|
||||
});
|
||||
// interiorFrac is retained in the options for callers that want the old
|
||||
// behaviour; the line-first classifier makes it unnecessary here.
|
||||
void interiorFrac;
|
||||
|
||||
// Lay labels per edge. Left/right stack along y; top/bottom along x
|
||||
// (with wider spacing — their labels are horizontal text).
|
||||
|
|
@ -217,20 +275,15 @@ export function layoutInspectorPins(
|
|||
};
|
||||
});
|
||||
}
|
||||
for (const [c, i] of byEdge('interior')) {
|
||||
// Interior pins keep their label ON the dot: the dialog renders a
|
||||
// tooltip instead of printed text (a label in the middle of the art
|
||||
// would sit on top of the silkscreen).
|
||||
laid[i] = {
|
||||
name: c.pin.name,
|
||||
edge: 'interior',
|
||||
dotX: c.x,
|
||||
dotY: c.y,
|
||||
labelX: c.x,
|
||||
labelY: c.y,
|
||||
needsLeader: false,
|
||||
signalKind: signalKindOf(c.pin),
|
||||
};
|
||||
// A leader is also needed whenever the DOT is inset from its gutter, even
|
||||
// if the label sits on the dot's own row: without it an inset column's
|
||||
// labels look unattached to anything.
|
||||
for (const p of laid) {
|
||||
if (!p) continue;
|
||||
if (p.edge === 'left' && p.dotX > 6) p.needsLeader = true;
|
||||
else if (p.edge === 'right' && p.dotX < W - 6) p.needsLeader = true;
|
||||
else if (p.edge === 'top' && p.dotY > 6) p.needsLeader = true;
|
||||
else if (p.edge === 'bottom' && p.dotY < H - 6) p.needsLeader = true;
|
||||
}
|
||||
|
||||
const has = (e: PinEdge) => classified.some((c) => c.edge === e);
|
||||
|
|
|
|||
Loading…
Reference in New Issue