Clarify component review scope alongside full comp crops

This commit is contained in:
Paul Bakaus
2026-09-20 10:49:03 -07:00
parent 37754ce3e1
commit bc426dd34f
4 changed files with 53 additions and 7 deletions
+16 -2
View File
@@ -1,5 +1,5 @@
import { describe, expect, test } from 'bun:test';
import { reviewUnits, decisionTargets, nextUnreviewed, approveRemaining, newDraft, submission, summarize, validBox, type ReviewPacket } from './model';
import { reviewScope, reviewUnits, decisionTargets, nextUnreviewed, approveRemaining, newDraft, submission, summarize, validBox, type ReviewPacket } from './model';
const packet: ReviewPacket = { id:'review-1', revision:'packet-1', title:'Test', round:1, comp:{url:'/comp.png',width:100,height:100}, components:[{id:'art',revision:'art-1',name:'Art',medium:'Raster',note:'',box:{x:0,y:0,w:1,h:1},preview:{kind:'image',url:'/art.png'}},{id:'control',revision:'control-1',name:'Button',medium:'HTML',note:'',box:{x:0,y:0,w:.1,h:.1},preview:{kind:'page',url:'/page.html'}}] };
describe('component review drafts',()=>{
test('bulk approval still requires explicit inventory confirmation',()=>{const draft=approveRemaining(packet,newDraft(packet));expect(summarize(packet,draft).canSubmit).toBe(false);draft.inventoryConfirmed=true;expect(submission(packet,draft).requestId).toBe(packet.id);});
@@ -96,7 +96,7 @@ test('review units collapse explicit code groups while retaining individual rast
draft.decisions['room-0']={revision:'control-1',action:'revise',feedback:'Fix this one',split:false};
const group=reviewUnits(p,draft)[0];expect(group.id).toBe('room-0');expect(group.representative.id).toBe('room-1');
for(const c of decisionTargets(p,draft,group.representative,true))draft.decisions[c.id]={revision:c.revision,action:'approve',feedback:'',split:false};
expect(draft.decisions['room-0'].feedback).toBe('Fix this one');expect(reviewUnits(p,draft)[0]).toMatchObject({pending:0,kind:'feedback',stateLabel:'1 need work'});
expect(draft.decisions['room-0'].feedback).toBe('Fix this one');expect(reviewUnits(p,draft)[0]).toMatchObject({pending:0,kind:'feedback',stateLabel:'1 needs work'});
expect(summarize(p,draft)).toMatchObject({approved:13,revisions:1,pending:1});
p.components[2].revision='changed';expect(reviewUnits(p,draft)[0]).toMatchObject({pending:1,kind:'pending'});
});
@@ -112,3 +112,17 @@ test('group approval cannot overwrite a reviewed representative',()=>{
expect(decisionTargets(p,draft,original,true).map(c=>c.id)).toEqual(['second']);
expect(decisionTargets(p,draft,original,false).map(c=>c.id)).toEqual([original.id]);
});
test('scope distinguishes other mapped pieces from explicit capture exclusions',()=>{
const p=structuredClone(packet), card=p.components[0];
card.note='Card outline';
const child=p.components[1];
child.box={x:.2,y:.2,w:.2,h:.2};
p.components.push({...child,id:'background',box:{x:0,y:0,w:1,h:1}});
expect(reviewScope(p,card).related.map(c=>c.id)).toEqual(['control']);
expect(reviewScope(p,card).excluded).toEqual([]);
card.preview.isolation={method:'dom-component-v1',selector:'#card',excludedComponents:['control']};
expect(reviewScope(p,card).excluded.map(c=>c.id)).toEqual(['control']);
expect(reviewScope(p,child).related).toEqual([]);
expect(reviewScope(p,card).description).toBe('Card outline');
});
+18
View File
@@ -133,3 +133,21 @@ export function reviewUnits(packet: ReviewPacket, draft: Draft, history?: Review
: feedback.length ? (members.length>1 ? `${feedback.length} ${feedback.length===1?'needs':'need'} work` : 'Feedback ready') : 'Approved'}];
});
}
/** Nearby map geometry is context, not proof of capture exclusions. Only the
* capture's explicit exclusion list can identify deliberately hidden layers. */
export function reviewScope(packet: ReviewPacket, component: Component) {
const excluded = new Set(component.preview.isolation?.excludedComponents ?? []);
const b = component.box;
const related = packet.components.filter(other => {
if (other.id === component.id) return false;
if (excluded.has(other.id)) return true;
const o = other.box;
const overlap = Math.max(0, Math.min(b.x+b.w,o.x+o.w)-Math.max(b.x,o.x)) *
Math.max(0, Math.min(b.y+b.h,o.y+o.h)-Math.max(b.y,o.y));
// Strictly smaller, substantially contained regions; omit backgrounds and
// neighboring cards. This never changes the review or any reference pixels.
return o.w*o.h < b.w*b.h && overlap / (o.w*o.h) >= .98;
});
return {description:component.note.trim(), related, excluded:related.filter(c=>excluded.has(c.id))};
}
+13 -5
View File
@@ -1,4 +1,4 @@
import { reviewUnits, reviewPeers, decisionTargets, inReviewQueue, type InventoryFilter, componentPresentation, nextUnreviewed, approveRemaining, componentState, repairStatus, newDraft, submission, summarize, type Box, type Decision, type Draft, type ReviewPacket, type ReviewHistory } from './model';
import { reviewScope, type Component, reviewUnits, reviewPeers, decisionTargets, inReviewQueue, type InventoryFilter, componentPresentation, nextUnreviewed, approveRemaining, componentState, repairStatus, newDraft, submission, summarize, type Box, type Decision, type Draft, type ReviewPacket, type ReviewHistory } from './model';
import { comparisonSize, hoverPan } from './viewport';
import { styles } from './styles';
import { icon } from './icons';
@@ -50,6 +50,13 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op
let resize: ResizeObserver | null = null;
const checkIcon = '<svg viewBox="0 0 16 16" aria-hidden="true"><path d="m3 8 3 3 7-7"/></svg>';
const feedbackIcon = '<svg viewBox="0 0 16 16" aria-hidden="true"><path d="m3 11 1 2 2-1 7-7-3-3-7 7v2Z"/></svg>';
const scopeMarkup = (p: ReviewPacket, item: Component) => {
if (p.stage === 'hero') return '';
const scope = reviewScope(p,item);
if (!scope.related.length) return '';
const names = scope.related.map(other=>esc(other.name)).join(' · ');
return `<div class="review-scope" aria-label="Review scope"><p><strong>Reviewing</strong> ${esc(scope.description || item.name)}</p><p class="separate-reviews"><strong>Separate review items</strong> ${names}</p>${scope.excluded.length?`<p class="scope-excluded">Hidden in this preview: ${scope.excluded.map(other=>esc(other.name)).join(' · ')}</p>`:''}</div>`;
};
const boxStyle = (b: Box) => `left:${pct(b.x)};top:${pct(b.y)};width:${pct(b.w)};height:${pct(b.h)}`;
function focusReview(id: string) {
root.getElementById(id)?.focus({preventScroll:true});
@@ -182,6 +189,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op
const useContext = !!(v?.context && outputMode === 'context');
const useFrame = v && (useContext ? v.context?.kind !== 'image' : v.preview.kind === 'page');
const sourceUrl = useContext && v?.context ? v.context.url : v?.preview.url;
const visibleScope = v ? reviewScope(vp,v) : null;
const materialLabel = presentation?.code ? `${presentation.label} · ${presentation.captured ? 'captured from code' : 'live preview'}` : v?.material ? `${v.material.alpha === 'transparent' ? 'Transparent' : v.material.alpha === 'opaque' ? 'Opaque' : 'Transparency unverified'} ${v.material.format}` : 'Raster · transparency unverified';
root.innerHTML = `<style>${styles}</style><section class="review ${assembled?'assembled-review':''}" aria-label="${assembled?'Assembled page review':'Component review'}" style="--comp-background:${/^#[0-9a-f]{6}$/i.test(packet.comp.background ?? '') ? packet.comp.background : '#eeeeee'}">
<header><div><h1>${submitted?'Review record.':assembled?'Review the assembled page.':'Review the components.'}</h1><p>${esc(packet.title)} <span>· Round ${packet.round}</span></p></div>${submitted ? '<span class="badge">Submitted · read-only</span>' : options.preview ? '<span class="badge">Interactive preview</span>' : ''}</header>
@@ -212,14 +220,14 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op
</div>`:''}
<div class="comparison-slot"><div class="comparison-panel ${groupOverview?'group-overview':''}"><h2 class="expanded-title">${esc(groupOverview?unit!.label:v!.name)}</h2>${peers.length>1?`<div class="review-peers"><strong>${peers.length} instances</strong>${!groupOverview?'<button id="all-instances" class="quiet">All instances</button>':'<span class="group-hint">Select to inspect</span>'}</div>`:''}<div class="compare-toolbar">${priorComponent?`<div class="round-switch" role="group" aria-label="Preview version"><button id="current-round" aria-label="Current · round ${packet.round}" title="Current · round ${packet.round}" aria-pressed="${!viewingPrevious}">Current</button><button id="previous-round" aria-label="Previous · round ${history!.packet.round}" title="Previous · round ${history!.packet.round}" aria-pressed="${viewingPrevious}">Previous</button></div>`:''}<label class="zoom-control" title="Comparison zoom · based on comp pixels">${icon('zoom')}<select id="zoom" aria-label="Comparison zoom">${[['fit','Fit'],['1','100%'],['2','200%'],['4','400%']].map(([value,label])=>`<option value="${value}" ${String(zoom)===value?'selected':''}>${label}</option>`).join('')}</select>${icon('chevronDown')}</label><button id="overlay" class="overlay-control" aria-label="Overlay comp" title="Overlay approved comp" aria-pressed="${overlay}"><svg viewBox="0 0 20 20" aria-hidden="true"><rect x="3" y="3" width="10" height="10"/><rect x="7" y="7" width="10" height="10"/></svg><span class="overlay-label">Overlay</span></button><div class="comparison-actions" role="group" aria-label="Comparison view actions"><button id="expand-comparison" class="icon-button" aria-label="${expandedComparison?'Restore comparison':'Enlarge comparison'}" title="${expandedComparison?'Restore comparison (Esc)':'Enlarge comparison'}" aria-expanded="${expandedComparison}">${icon(expandedComparison?'compact':'expand')}</button>${v?.preview.kind==='image'?`<a class="icon-button source-link" href="${url(sourceUrl!)}" target="_blank" rel="noopener" aria-label="${useContext?'Open context capture':presentation!.fileLabel}" title="${useContext?'Open context capture':presentation!.fileLabel}">${icon('external')}</a>`:''}</div></div>
${groupOverview?`<div class="instance-grid" aria-label="All instances of ${esc(unit!.label)}"><div class="instance-grid-labels"><span>In the comp</span><span>Produced</span></div>${unit!.members.map((m,n)=>{const st=stateFor(m);return `<button class="instance-row ${st.kind}" data-instance="${esc(m.id)}" aria-label="Inspect instance ${n+1}: ${esc(m.name)}${esc(st.label)}"><span class="instance-caption"><strong>${esc(m.name)}</strong><span>${esc(st.label)}</span></span><span class="instance-pair"><span class="instance-reference" style="width:min(100%,${m.box.w*packet.comp.width}px,${180*m.box.w*packet.comp.width/(m.box.h*packet.comp.height)}px);aspect-ratio:${m.box.w*packet.comp.width}/${m.box.h*packet.comp.height}"><img src="${url(packet.comp.url)}" alt="Comp: ${esc(m.name)}" loading="lazy" style="width:${100/m.box.w}%;left:${-100*m.box.x/m.box.w}%;top:${-100*m.box.y/m.box.h}%"></span><span class="instance-produced" style="width:min(100%,${m.box.w*packet.comp.width}px,${180*m.box.w*packet.comp.width/(m.box.h*packet.comp.height)}px);aspect-ratio:${m.box.w*packet.comp.width}/${m.box.h*packet.comp.height}">${m.preview.kind==='image'?`<img src="${url(m.preview.url)}" alt="Produced: ${esc(m.name)}" loading="lazy">`:m.thumbnail?`<img src="${url(m.thumbnail.url)}" alt="Preview: ${esc(m.name)}" loading="lazy">`:'Open live component'}</span></span></button>`;}).join('')}</div>`:''}
<div class="compare">
<figure><figcaption>${viewingPrevious ? `Comp · Round ${history!.packet.round}` : assembled ? 'Approved comp' : 'In the comp'}</figcaption><div class="pan-viewport" aria-label="Reference comparison canvas" tabindex="0"><div class="crop-stage"><img class="crop-image" src="${url(vp.comp.url)}" alt="Reference region for ${esc(v!.name)}" style="width:${100/v!.box.w}%;left:${-100*v!.box.x/v!.box.w}%;top:${-100*v!.box.y/v!.box.h}%"></div></div></figure>
${groupOverview?`<div class="instance-grid" aria-label="All instances of ${esc(unit!.label)}"><div class="instance-grid-labels"><span>Full comp crop</span><span>Component preview</span></div>${unit!.members.map((m,n)=>{const st=stateFor(m);return `<button class="instance-row ${st.kind}" data-instance="${esc(m.id)}" aria-label="Inspect instance ${n+1}: ${esc(m.name)}${esc(st.label)}"><span class="instance-caption"><strong>${esc(m.name)}</strong><span>${esc(st.label)}</span></span><span class="instance-pair"><span class="instance-reference" style="width:min(100%,${m.box.w*packet.comp.width}px,${180*m.box.w*packet.comp.width/(m.box.h*packet.comp.height)}px);aspect-ratio:${m.box.w*packet.comp.width}/${m.box.h*packet.comp.height}"><img src="${url(packet.comp.url)}" alt="Comp: ${esc(m.name)}" loading="lazy" style="width:${100/m.box.w}%;left:${-100*m.box.x/m.box.w}%;top:${-100*m.box.y/m.box.h}%"></span><span class="instance-produced" style="width:min(100%,${m.box.w*packet.comp.width}px,${180*m.box.w*packet.comp.width/(m.box.h*packet.comp.height)}px);aspect-ratio:${m.box.w*packet.comp.width}/${m.box.h*packet.comp.height}">${m.preview.kind==='image'?`<img src="${url(m.preview.url)}" alt="Produced: ${esc(m.name)}" loading="lazy">`:m.thumbnail?`<img src="${url(m.thumbnail.url)}" alt="Preview: ${esc(m.name)}" loading="lazy">`:'Open live component'}</span></span>${scopeMarkup(packet,m)}</button>`;}).join('')}</div>`:''}
${!groupOverview&&v?scopeMarkup(vp,v):''}<div class="compare">
<figure><figcaption>${viewingPrevious ? `Comp · Round ${history!.packet.round}` : assembled ? 'Approved comp' : visibleScope?.related.length ? 'Full comp crop' : 'In the comp'}</figcaption><div class="pan-viewport" aria-label="Reference comparison canvas" tabindex="0"><div class="crop-stage"><img class="crop-image" src="${url(vp.comp.url)}" alt="Reference region for ${esc(v!.name)}" style="width:${100/v!.box.w}%;left:${-100*v!.box.x/v!.box.w}%;top:${-100*v!.box.y/v!.box.h}%"></div></div></figure>
<figure><figcaption>${viewingPrevious ? `Previous · Round ${history!.packet.round}` : assembled ? 'Assembled page' : useContext ? 'In context' : history ? `${presentation!.caption} · Round ${packet.round}` : presentation!.caption}</figcaption><div class="pan-viewport" aria-label="Produced comparison canvas" tabindex="0"><div class="output crop-stage ${hasTransparency&&!useContext&&!useFrame&&backdrop==='checker'?'checker':''}">${!useFrame ? `<img class="asset" src="${url(sourceUrl!)}" alt="Produced ${esc(v!.name)}" style="object-position:${esc(v!.preview.position ?? 'center')}">` : `<iframe aria-hidden="true" title="Rendered ${esc(v!.name)}" src="${url(sourceUrl!)}" sandbox="" tabindex="-1" width="${vp.comp.width}" height="${vp.comp.height}"></iframe>`}${overlay ? `<img class="crop-image overlay-image" src="${url(vp.comp.url)}" alt="Reference overlay" style="width:${100/v!.box.w}%;left:${-100*v!.box.x/v!.box.w}%;top:${-100*v!.box.y/v!.box.h}%">` : ''}</div></div></figure>
</div>
${hasTransparency || v!.context ? `<div class="view-controls">${v!.context ? `<div role="group" aria-label="Component view"><button id="isolated" aria-pressed="${!useContext}">${isRaster?'Asset only':'Component only'}</button><button id="context" aria-pressed="${useContext}">In context</button></div>` : ''}${hasTransparency?`<div class="background-options" role="group" aria-label="Asset preview background"><button id="background-checker" class="swatch-button" aria-label="Checkerboard background" title="Checkerboard background" aria-pressed="${backdrop==='checker'}" ${useContext?'disabled':''}><span class="background-swatch checker"></span></button><button id="background-page" class="swatch-button" aria-label="${vp.comp.background?'Page color':'Neutral'} background" title="${vp.comp.background?'Page color':'Neutral'} background" aria-pressed="${backdrop==='page'}" ${useContext?'disabled':''}><span class="background-swatch page-swatch"></span></button></div>`:''}</div>` : ''}
</div></div>${!assembled&&!groupOverview?`<div class="component-details"><div class="material">${icon(presentation!.code ? 'code' : 'image')}<strong>${esc(materialLabel)}</strong><span>${v?.material ? `${v.material.width} × ${v.material.height} px` : ''}</span></div>${vp.stage==='components'&&presentation?.captured&&!v?.preview.isolation?'<p class="layering">Legacy region capture · may include overlapping components.</p>':''}${v?.context?.layering&&(isRaster||useContext)?`<p class="layering">${esc(v.context.layering)}</p>`:''}
<p class="component-note">${esc(v!.note)}</p>
<p class="component-note">${!visibleScope?.related.length?esc(v!.note):''}</p>
</div>`:''}</div><div class="review-form">${notice}${viewingPrevious?'<p class="previous-notice">Viewing the previous round. Return to Current to make a decision.</p>':''}${submitted?`<div class="record-verdict"><strong>${d?.action==='approve'?'Approved':d?.action==='revise'?'Changes requested':'Not reviewed'}</strong><span>Submitted in round ${packet.round} · read-only</span></div>`:`<div class="decisions" role="group" aria-label="Decision for ${esc(c.name)}">${!assembled?`<div class="decision-title"><strong>Your review <span>Round ${packet.round}</span></strong>${viewingPrevious?'<p>Return to Current to review this round.</p>':''}</div>`:''}<button id="approve" ${!targets.length?'disabled':''} class="decision-approve ${d?.action === 'approve' ? 'approved' : ''}" aria-pressed="${d?.action === 'approve'}">${assembled?(sending?'Sending…':'Approve & continue'):targets.length>1?`Approve ${targets.length} instances`:'Looks good'}</button><button id="revise" ${!targets.length?'disabled':''} class="decision-revise ${d?.action === 'revise' ? 'revise' : ''}" aria-pressed="${d?.action === 'revise'}">${targets.length>1?`Revise ${targets.length} instances`:'Needs work'}</button>${d && !assembled && !groupOverview ? `<button id="clear" class="quiet icon-button" aria-label="Clear decision" title="Clear decision">${icon('undo')}</button>` : ''}</div>`}
${edit ? `<form id="feedback-form"><div class="feedback-fields"><label class="feedback-field">What needs to change?<textarea id="feedback" aria-describedby="feedback-hint">${esc(edit.feedback)}</textarea></label><p id="feedback-hint" class="feedback-hint">Optional — leave blank for the agent to diagnose.</p>${!assembled?`<label class="check"><input id="split" type="checkbox" ${edit.split ? 'checked' : ''}> Split into separately reviewable components</label>`:''}</div><div class="feedback-actions"><button id="cancel-feedback" type="button" class="quiet">Cancel</button><button id="save-feedback" type="submit" class="primary">${assembled?'Send feedback':isLast?'Save & finish review':'Save & next'} <svg viewBox="0 0 24 24" aria-hidden="true"><path d="M4 12h15m-6-6 6 6-6 6"/></svg></button><span class="shortcut-hint">${shortcutLabel}</span></div></form>` : d?.action==='revise' ? `<p class="saved-feedback">${esc(d.feedback || 'No note — agent will diagnose.')}</p>` : ''}
${assembled?`<p class="page-review-status" role="status">${esc(error || (submitted?'Your decision is saved.':sending?'Sending…':edit?'':'Approval confirms the composition and that nothing is missing.'))}</p>`:''}</div>` : missing ? `<p>This piece will be added to the unresolved inventory.</p><label class="feedback-field">Name<input id="missing-name" value="${esc(missing.name)}"></label><label class="feedback-field">What is missing?<textarea id="missing-feedback">${esc(missing.feedback)}</textarea></label><div class="coordinates">${(['x','y','w','h'] as const).map(k=>`<label>${{x:'Left',y:'Top',w:'Width',h:'Height'}[k]} %<input type="number" data-coordinate="${k}" value="${Math.round(missing.box[k]*1000)/10}" min="0" max="100" step="0.1"></label>`).join('')}</div><button id="remove-missing">Remove this mark</button></div>` : '<p>No components supplied.</p></div>'}
+6
View File
@@ -128,4 +128,10 @@ export const styles = `
#comparison-dialog .group-overview{overflow:auto;min-height:0}
#comparison-dialog .group-overview>.compare-toolbar{top:0}
#comparison-dialog .group-overview>.expanded-title{padding-right:44px}
.review-scope{font-size:12px;line-height:1.5;margin:8px 0 12px;color:var(--color-text,#292929);text-align:left;white-space:normal}
.review-scope p{margin:4px 0;overflow-wrap:anywhere}.review-scope strong{font-weight:600;margin-right:5px}
.review-scope .separate-reviews,.review-scope .scope-excluded{font-size:11px;color:var(--muted)}
.instance-row .review-scope{margin:12px 0 0}
#comparison-dialog .review-scope{flex:none}
`;