From 37754ce3e1d9b1c77edd5b5d43209190c85c4b7a Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Fri, 18 Sep 2026 18:09:25 -0700 Subject: [PATCH] Group repeated component reviews by default --- skill/reference/component-review.md | 2 +- ui/component-review/model.test.ts | 28 ++++++++++++- ui/component-review/model.ts | 29 ++++++++++++- ui/component-review/review.ts | 64 +++++++++++++++++------------ ui/component-review/styles.ts | 26 ++++++++++++ 5 files changed, 118 insertions(+), 31 deletions(-) diff --git a/skill/reference/component-review.md b/skill/reference/component-review.md index 0864ba1be..320ef28b1 100644 --- a/skill/reference/component-review.md +++ b/skill/reference/component-review.md @@ -6,7 +6,7 @@ Use this checkpoint on comp-led builds after producing the initial component kit Keep the measured spec's region IDs. Include every visible region: produced raster assets and working HTML/CSS/SVG for text, controls, patterns, decoration and layout elements. A region rendered in code needs an actual review document, not a promise to implement it later. Use semantic HTML for content and controls. Do not flatten the page or combine unrelated regions to avoid review. Report omitted regions so the user can mark what is missing. -For a repeated code pattern, give instances of the same component and role the same `reviewGroup` name. Group peers of the same kind, not a container with its contents or unrelated text roles. Keep every instance and its region ID in the manifest, in the same kit document. The user can inspect instances and explicitly apply one decision to the unreviewed group. Unique raster assets still require their own review; grouping never removes inventory or gate checks. +For a repeated code pattern, give instances of the same component and role the same `reviewGroup` name. Group peers of the same kind, not a container with its contents or unrelated text roles. Keep every instance and its region ID in the manifest, in the same kit document. The review opens with one item per group and shows its instances together. One explicit group decision applies to its unreviewed instances; the user can open an instance to leave an exception. Existing decisions are preserved. Unique raster assets still require their own review; grouping never removes inventory or gate checks. Before producing assets, inspect each reference crop against its named subject. Coarse grid cells and automatic ink snapping can include neighbors or omit parts of a compound element. Correct the measured region with an explicit normalized `box`; do not build to a known bad crop. Check the code preview contains the complete component before presenting it. The capture tool refuses content cut off by the review crop. diff --git a/ui/component-review/model.test.ts b/ui/component-review/model.test.ts index 374b5a792..6380a95ec 100644 --- a/ui/component-review/model.test.ts +++ b/ui/component-review/model.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'bun:test'; -import { nextUnreviewed, approveRemaining, newDraft, submission, summarize, validBox, type ReviewPacket } from './model'; +import { 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);}); @@ -86,3 +86,29 @@ test('explicit pattern decisions preserve prior decisions, raster reviews and op expect(draft.decisions.b.feedback).toBe('Keep this specific repair'); expect(reviewPeers(p,packet.components[0])).toEqual([packet.components[0]]); }); + + +test('review units collapse explicit code groups while retaining individual raster assets',()=>{ + const p:ReviewPacket={...packet,components:[...Array.from({length:14},(_,i)=>({...packet.components[1],id:`room-${i}`,reviewGroup:'room-name'})),{...packet.components[0],reviewGroup:'room-name'}]}; + const draft=newDraft(p);const units=reviewUnits(p,draft); + expect(units).toHaveLength(2);expect(units[0].members).toHaveLength(14);expect(units[1].members).toHaveLength(1); + expect(units[0].label).toBe('Room name');expect(units[0].pending).toBe(14); + 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(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'}); +}); + +test('group approval cannot overwrite a reviewed representative',()=>{ + const p=structuredClone(packet); + p.components=p.components.slice(0,1); + const original=p.components[0]; + original.reviewGroup='repeated'; original.preview={kind:'page',url:'/component.html'}; + p.components.push({...original,id:'second'}); + const draft=newDraft(p); + draft.decisions[original.id]={revision:original.revision,action:'revise',feedback:'Preserve this exception',split:false}; + 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]); +}); diff --git a/ui/component-review/model.ts b/ui/component-review/model.ts index 0cdaed269..c1e6e2f8a 100644 --- a/ui/component-review/model.ts +++ b/ui/component-review/model.ts @@ -105,6 +105,31 @@ export function reviewPeers(packet: ReviewPacket, component: Component): Compone return packet.components.filter(c => c.reviewGroup === component.reviewGroup && componentPresentation(c).code); } export function decisionTargets(packet: ReviewPacket, draft: Draft, selected: Component, grouped: boolean, editing: string[] = []) { - return (grouped ? reviewPeers(packet, selected) : [selected]).filter(c => c.id === selected.id || - (componentState(c, draft).kind === 'pending' && !editing.includes(c.id))); + const peers = grouped ? reviewPeers(packet, selected) : [selected]; + if (peers.length === 1) return [selected]; + return peers.filter(c => componentState(c, draft).kind === 'pending' && + (c.id === selected.id || !editing.includes(c.id))); +} + + +/** One visible review unit per explicitly authored code pattern. Receipts stay + * per instance, including revision checks and individual exceptions. */ +export function reviewUnits(packet: ReviewPacket, draft: Draft, history?: ReviewHistory | null) { + const seen = new Set(); + return packet.components.flatMap(component => { + if (seen.has(component.id)) return []; + const members = reviewPeers(packet, component); + members.forEach(c => seen.add(c.id)); + const pending = members.filter(c => componentState(c,draft,history).kind === 'pending'); + const feedback = members.filter(c => componentState(c,draft,history).kind === 'feedback'); + const representative = pending[0] ?? feedback[0] ?? component; + const kind = pending.length ? 'pending' as const : feedback.length ? 'feedback' as const : 'approved' as const; + return [{id:component.id, members, representative, pending:pending.length, kind, + label:members.length > 1 ? component.reviewGroup!.replace(/[-_]+/g,' ').replace(/^./,c=>c.toUpperCase()) : component.name, + box: {x:Math.min(...members.map(c=>c.box.x)),y:Math.min(...members.map(c=>c.box.y)), + w:Math.max(...members.map(c=>c.box.x+c.box.w))-Math.min(...members.map(c=>c.box.x)), + h:Math.max(...members.map(c=>c.box.y+c.box.h))-Math.min(...members.map(c=>c.box.y))}, + stateLabel:pending.length ? (members.length>1 ? `${pending.length} to review` : componentState(representative,draft,history).label) + : feedback.length ? (members.length>1 ? `${feedback.length} ${feedback.length===1?'needs':'need'} work` : 'Feedback ready') : 'Approved'}]; + }); } diff --git a/ui/component-review/review.ts b/ui/component-review/review.ts index a81247abe..df24a8467 100644 --- a/ui/component-review/review.ts +++ b/ui/component-review/review.ts @@ -1,4 +1,4 @@ -import { 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 { 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'; @@ -28,7 +28,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op const edits: Record = {}; let finished = !!options.completed || !summarize(packet,draft).pending; let lastDecision: {id: string; name: string; action: 'approve' | 'revise'; previous: Record} | null = null; - let applyGroup = false; + let applyGroup = true; const shortcutLabel = /Mac|iPhone|iPad/.test(navigator.platform) ? '⌘Enter' : 'Ctrl+Enter'; let overlay = false; let expandedComparison = false; @@ -58,7 +58,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op const next = nextUnreviewed(packet, draft, after); finished = !next; if (next) selected = next; - mobilePane='component'; previousRound=false; overlay=false; zoom='fit'; outputMode='isolated'; applyGroup=false; + mobilePane='component'; previousRound=false; overlay=false; zoom='fit'; outputMode='isolated'; applyGroup=true; inventoryFilter = finished ? 'reviewed' : 'pending'; const showNext = () => { render(); @@ -79,7 +79,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op function updateDecision(action: 'approve' | 'revise') { if (sending || submitted || previousRound || closingComparison) return; const c = packet.components.find(c => c.id === selected); - if (!c) return; + if (!c || !decisionTargets(packet,draft,c,applyGroup,Object.keys(edits)).length) return; const saved = draft.decisions[c.id]; const current = saved?.revision===c.revision ? saved : undefined; const targets = decisionTargets(packet,draft,c,applyGroup,Object.keys(edits)); @@ -98,7 +98,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op function beginFeedback() { if (sending || submitted || previousRound || closingComparison) return; const c = packet.components.find(c=>c.id===selected); - if (!c) return; + if (!c || !decisionTargets(packet,draft,c,applyGroup,Object.keys(edits)).length) return; const saved = draft.decisions[c.id]; const current = saved?.revision===c.revision ? saved : undefined; edits[c.id] ??= {feedback:current?.feedback ?? '',split:current?.split ?? false}; @@ -135,8 +135,11 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op renderedSelection=selected;renderedZoom=zoom; const c = packet.components.find(c => c.id === selected); const missing = draft.missing.find(m => m.id === selected); - const box = c?.box ?? missing?.box; - const index = c ? packet.components.indexOf(c) + 1 : packet.components.length + draft.missing.findIndex(m => m.id === selected) + 1; + const units = reviewUnits(packet,draft,options.history); + const unit = units.find(u=>u.members.some(m=>m.id===selected)); + const groupOverview = !!unit && unit.members.length>1 && applyGroup; + const box = groupOverview ? unit!.box : c?.box ?? missing?.box; + const index = unit ? units.indexOf(unit)+1 : units.length + draft.missing.findIndex(m => m.id === selected)+1; const savedDecision = c ? draft.decisions[c.id] : undefined; const d = savedDecision?.revision === c?.revision ? savedDecision : undefined; const edit = c ? edits[c.id] : undefined; @@ -160,8 +163,11 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op const state=componentState(item,draft,history); return submitted&&state.kind==='feedback'?{...state,label:'Changes requested'}:state; }; - const reviewedCount = stats.approved+stats.revisions+draft.missing.length; - const shownComponents = (inventoryFilter==='pending'?orderedComponents():packet.components).filter(item=>inReviewQueue(item,draft,inventoryFilter)); + const pendingUnits = units.filter(u=>u.kind==='pending').length; + const reviewedCount = units.length-pendingUnits+draft.missing.length; + const shownUnits=units.filter(u=>inventoryFilter==='all'||(u.kind==='pending')===(inventoryFilter==='pending')); + const shownComponents=shownUnits.map(u=>u.representative); + const displayName = (id:string) => units.find(u=>u.members.some(c=>c.id===id))?.label ?? id; const summaryDetails = [ stats.revisions+draft.missing.length ? `${stats.revisions+draft.missing.length} feedback ready` : '', carriedCount ? `${carriedCount} ${carriedCount===1?'approval':'approvals'} kept` : '', @@ -169,7 +175,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op addedCount ? `${addedCount} added` : '', history?.removed.length ? `${history.removed.length} removed` : '', ].filter(Boolean).join(' · '); - const statusMessage = error || (uncommitted ? 'Save or cancel your open feedback before sending.' : submitted ? (options.preview ? 'Preview submitted. No run changed.' : 'Review submitted.') : stats.hasFeedback ? 'Ready to send for corrections.' : stats.pending ? `${stats.pending} left to review` : !draft.inventoryConfirmed ? 'Confirm the map is complete.' : 'Ready to continue.'); + const statusMessage = error || (uncommitted ? 'Save or cancel your open feedback before sending.' : submitted ? (options.preview ? 'Preview submitted. No run changed.' : 'Review submitted.') : stats.hasFeedback ? 'Ready to send for corrections.' : stats.pending ? `${pendingUnits} left to review` : !draft.inventoryConfirmed ? 'Confirm the map is complete.' : 'Ready to continue.'); const presentation = v ? componentPresentation(v) : null; const isRaster = v?.preview.kind === 'image' && !presentation?.code; const hasTransparency = isRaster || v?.material?.alpha === 'transparent'; @@ -180,7 +186,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op root.innerHTML = `

${submitted?'Review record.':assembled?'Review the assembled page.':'Review the components.'}

${esc(packet.title)} · Round ${packet.round}

${submitted ? 'Submitted · read-only' : options.preview ? 'Interactive preview' : ''}
${options.preview ? '

Historical hotel artwork for testing this interface. Decisions stay in this preview; no run is changed.

' : ''} - ${history && !assembled ? `

${stats.pending} ${stats.pending===1?'component':'components'} to review${summaryDetails}

${stats.pending?``:''}${history.removed.length?`
Removed from the map

${history.removed.map(item=>esc(item.name)).join(' · ')}. Confirm these omissions are intentional before accepting the map.

`:''}
`:''} + ${history && !assembled ? `

${pendingUnits} ${pendingUnits===1?'item':'items'} to review${summaryDetails}

${stats.pending?``:''}${history.removed.length?`
Removed from the map

${history.removed.map(item=>esc(item.name)).join(' · ')}. Confirm these omissions are intentional before accepting the map.

`:''}
`:''} ${!assembled?`
`:''}
${!assembled?`
@@ -188,8 +194,9 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op
Approved composition for ${esc(packet.title)} ${box && !finished ? `
` : ''} - ${packet.components.map((item,i) => {const state=stateFor(item);return ``}).join('')} - ${draft.missing.map((item,i)=>``).join('')} + ${units.map((u,i) => {const item=u.representative;const state={kind:u.kind,label:u.stateLabel};const active=!finished&&unit?.id===u.id;return ``}).join('')} + ${groupOverview&&!finished?unit!.members.map(m=>`
`).join(''):''} + ${draft.missing.map((item,i)=>``).join('')}
@@ -197,28 +204,29 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op ${marking ? '
Draw around the missing piece.
' : ''}
`:''}
- ${!assembled?`

${finished ? 'Review summary' : `${index} ${esc(c?.name ?? missing?.name ?? 'Component')}`}

`:''}
- ${finished && !assembled ? `

${submitted ? 'Review sent.' : 'All components reviewed.'}

${stats.approved} approved · ${stats.revisions} flagged for repair${draft.missing.length ? ` · ${draft.missing.length} missing` : ''}

${submitted ? 'Your decisions are saved.' : stats.hasFeedback ? 'Send your feedback to start the next repair round.' : 'Confirm nothing is missing, then approve and continue.'}

${packet.components.map(item=>{const decision=draft.decisions[item.id];const state=stateFor(item);return ``;}).join('')}${draft.missing.map(item=>``).join('')}
${notice?`
${notice}
`:''}` : c ? ` + ${!assembled?`

${finished ? 'Review summary' : `${index} ${esc(groupOverview ? unit!.label : c?.name ?? missing?.name ?? 'Component')}`}

`:''}
+ ${finished && !assembled ? `

${submitted ? 'Review sent.' : 'All components reviewed.'}

${stats.approved} approved · ${stats.revisions} flagged for repair${draft.missing.length ? ` · ${draft.missing.length} missing` : ''}

${submitted ? 'Your decisions are saved.' : stats.hasFeedback ? 'Send your feedback to start the next repair round.' : 'Confirm nothing is missing, then approve and continue.'}

${units.map(u=>{const item=u.representative;const decision=draft.decisions[item.id];const state=stateFor(item);return ``;}).join('')}${draft.missing.map(item=>``).join('')}
${notice?`
${notice}
`:''}` : c ? ` ${history ? `
${viewingPrevious&&repair?.prior?.action==='revise'?`

Previous feedback · Round ${repair.feedbackRound}

${esc(repair.prior.feedback || 'No written feedback was supplied.')}
${repair.prior.split?'

Requested: split into separately reviewable components.

':''}
`:repair?.carried?`

Unchanged · approval kept

`:''} ${repair?.change?.kind==='changed'?`
${repair.change.files.length?`${repair.change.files.length} changed ${repair.change.files.length===1?'file':'files'}`:repair.change.reasons.includes('region')?'Region changed':priorComponent?.note!==c.note?'Description changed · files unchanged':'Component definition changed · files unchanged'}${repair.change.files.length?`
    ${repair.change.files.map(path=>`
  • ${esc(path)}
    • `).join('')}
`:''}${priorComponent&&priorComponent.note!==c.note?`
Previous description
${esc(priorComponent.note)}
Current description
${esc(c.note)}
`:''}
`:''}
`:''} -

${esc(v!.name)}

${peers.length>1?`
${esc(c!.reviewGroup!)} · ${peers.length} instances
${peers.map(peer=>``).join('')}
`:''}
${priorComponent?`
`:''}
${v?.preview.kind==='image'?`${icon('external')}`:''}
+

${esc(groupOverview?unit!.label:v!.name)}

${peers.length>1?`
${peers.length} instances${!groupOverview?'':'Select to inspect'}
`:''}
${priorComponent?`
`:''}
${v?.preview.kind==='image'?`${icon('external')}`:''}
+ ${groupOverview?`
In the compProduced
${unit!.members.map((m,n)=>{const st=stateFor(m);return ``;}).join('')}
`:''}
${viewingPrevious ? `Comp · Round ${history!.packet.round}` : assembled ? 'Approved comp' : 'In the comp'}
Reference region for ${esc(v!.name)}
${viewingPrevious ? `Previous · Round ${history!.packet.round}` : assembled ? 'Assembled page' : useContext ? 'In context' : history ? `${presentation!.caption} · Round ${packet.round}` : presentation!.caption}
${!useFrame ? `Produced ${esc(v!.name)}` : ``}${overlay ? `Reference overlay` : ''}
${hasTransparency || v!.context ? `
${v!.context ? `
` : ''}${hasTransparency?`
`:''}
` : ''} -
${!assembled?`
${icon(presentation!.code ? 'code' : 'image')}${esc(materialLabel)}${v?.material ? `${v.material.width} × ${v.material.height} px` : ''}
${vp.stage==='components'&&presentation?.captured&&!v?.preview.isolation?'

Legacy region capture · may include overlapping components.

':''}${v?.context?.layering&&(isRaster||useContext)?`

${esc(v.context.layering)}

`:''} +
${!assembled&&!groupOverview?`
${icon(presentation!.code ? 'code' : 'image')}${esc(materialLabel)}${v?.material ? `${v.material.width} × ${v.material.height} px` : ''}
${vp.stage==='components'&&presentation?.captured&&!v?.preview.isolation?'

Legacy region capture · may include overlapping components.

':''}${v?.context?.layering&&(isRaster||useContext)?`

${esc(v.context.layering)}

`:''}

${esc(v!.note)}

-
`:''}
${notice}${viewingPrevious?'

Viewing the previous round. Return to Current to make a decision.

':''}${submitted?`
${d?.action==='approve'?'Approved':d?.action==='revise'?'Changes requested':'Not reviewed'}Submitted in round ${packet.round} · read-only
`:`
${!assembled?`
Your review Round ${packet.round}${viewingPrevious?'

Return to Current to review this round.

':''}
`:''}${d && !assembled ? `` : ''}
`} +
`:''}
${notice}${viewingPrevious?'

Viewing the previous round. Return to Current to make a decision.

':''}${submitted?`
${d?.action==='approve'?'Approved':d?.action==='revise'?'Changes requested':'Not reviewed'}Submitted in round ${packet.round} · read-only
`:`
${!assembled?`
Your review Round ${packet.round}${viewingPrevious?'

Return to Current to review this round.

':''}
`:''}${d && !assembled && !groupOverview ? `` : ''}
`} ${edit ? `
` : d?.action==='revise' ? `

${esc(d.feedback || 'No note — agent will diagnose.')}

` : ''} ${assembled?`

${esc(error || (submitted?'Your decision is saved.':sending?'Sending…':edit?'':'Approval confirms the composition and that nothing is missing.'))}

`:''}
` : missing ? `

This piece will be added to the unresolved inventory.

${(['x','y','w','h'] as const).map(k=>``).join('')}
` : '

No components supplied.

'}
- ${!assembled?`

Components

-
${shownComponents.map(item=>{const i=packet.components.indexOf(item);const state=stateFor(item); return ``}).join('')}${(inventoryFilter==='pending'?[]:draft.missing).map((m,i)=>``).join('')}${!shownComponents.length&&(inventoryFilter==='pending'||!draft.missing.length)?`

${inventoryFilter==='pending'?'Nothing left to review. Your decisions are ready.':'No components reviewed yet.'}

`:''}
`:''} + ${!assembled?`

Components

+
${shownComponents.map(item=>{const u=units.find(u=>u.members.some(m=>m.id===item.id))!;const i=units.indexOf(u);const state={kind:u.kind,label:u.stateLabel}; return ``}).join('')}${(inventoryFilter==='pending'?[]:draft.missing).map((m,i)=>``).join('')}${!shownComponents.length&&(inventoryFilter==='pending'||!draft.missing.length)?`

${inventoryFilter==='pending'?'Nothing left to review. Your decisions are ready.':'No components reviewed yet.'}

`:''}
`:''} ${assembled?'':submitted?`
Round ${packet.round} submitted · read-only${stats.approved} approved · ${stats.revisions} changes requested
`:`
${!stats.pending&&!uncommitted ? `` : ''}

${esc(statusMessage)}

`} `; const comparisonDialog=root.querySelector('#comparison-dialog')!; @@ -243,9 +251,10 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op if (focusId) root.getElementById(focusId)?.focus({preventScroll:true}); else if(focusSelection) Array.from(root.querySelectorAll('.item[data-select]')).find(el=>el.dataset.select===focusSelection)?.focus({preventScroll:true}); const on = (id:string, action:()=>void) => root.querySelector(`#${id}`)?.addEventListener('click', action); - function selectComponent(id:string, enlarge=false) { + function selectComponent(id:string, enlarge=false, individual=false) { if(marking)return; - applyGroup=false; finished=false; selected=id; mobilePane='component'; overlay=false; zoom='fit'; outputMode='isolated'; previousRound=false; render(); + if(!individual)id=units.find(u=>u.members.some(m=>m.id===id))?.representative.id ?? id; + applyGroup=!individual; finished=false; selected=id; mobilePane='component'; overlay=false; zoom='fit'; outputMode='isolated'; previousRound=false; render(); // Use the newly rendered control so map shortcuts share the toolbar's // animation, focus management, reduced-motion and dismissal behavior. if(enlarge)root.querySelector('#expand-comparison')?.click(); @@ -253,12 +262,12 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op root.querySelectorAll('[data-select]').forEach(el => el.onclick = () => selectComponent(el.dataset.select!,el.classList.contains('pin')&&packet.components.some(item=>item.id===el.dataset.select))); on('previous-round',()=>{previousRound=true;render();}); on('current-round',()=>{previousRound=false;render();}); - on('review-changes',()=>{const pending=orderedComponents().filter(item=>stateFor(item).kind==='pending');const current=pending.findIndex(item=>item.id===selected);const next=pending[(current+1)%pending.length];if(next){finished=false;selected=next.id;mobilePane='component';inventoryFilter='pending';previousRound=false;zoom='fit';overlay=false;outputMode='isolated';render();}}); + on('review-changes',()=>{const pending=orderedComponents().filter(item=>stateFor(item).kind==='pending');const current=pending.findIndex(item=>item.id===selected);const next=pending[(current+1)%pending.length];if(next){applyGroup=true;finished=false;selected=next.id;mobilePane='component';inventoryFilter='pending';previousRound=false;zoom='fit';overlay=false;outputMode='isolated';render();}}); root.querySelectorAll('[data-filter]').forEach(button=>button.onclick=()=>{ inventoryFilter=button.dataset.filter as InventoryFilter;finished=!stats.pending&&inventoryFilter==='pending'; - const matches=orderedComponents().filter(item=>inReviewQueue(item,draft,inventoryFilter)); + const matches=units.filter(u=>inventoryFilter==='all'||(u.kind==='pending')===(inventoryFilter==='pending')).map(u=>u.representative); const selectedMissing=inventoryFilter!=='pending'&&draft.missing.some(item=>item.id===selected); - if(!selectedMissing&&!matches.some(item=>item.id===selected)&&matches.length){selected=matches[0].id;mobilePane='component';previousRound=false;zoom='fit';overlay=false;outputMode='isolated';} + if(!selectedMissing&&!matches.some(item=>item.id===selected)&&matches.length){applyGroup=true;selected=matches[0].id;mobilePane='component';previousRound=false;zoom='fit';overlay=false;outputMode='isolated';} render(); }); on('show-summary',()=>{finished=true;mobilePane='component';inventoryFilter='reviewed';render();focusReview('review-summary');}); @@ -276,7 +285,8 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op for(const [id,decision] of Object.entries(previous.previous)){if(decision)draft.decisions[id]=decision;else delete draft.decisions[id];} delete edits[previous.id];selected=previous.id;finished=false;previousRound=false;mobilePane='component';inventoryFilter='all';lastDecision=null;render();focusReview('approve'); }); - root.querySelector('#apply-group')?.addEventListener('change',e=>{applyGroup=(e.target as HTMLInputElement).checked;render();}); + on('all-instances',()=>{applyGroup=true;selected=unit!.representative.id;previousRound=false;render();}); + root.querySelectorAll('[data-instance]').forEach(el=>el.onclick=()=>selectComponent(el.dataset.instance!,false,true)); on('overlay',()=>{overlay=!overlay; render();}); on('isolated',()=>{outputMode='isolated';render();}); on('context',()=>{outputMode='context';render();}); @@ -346,7 +356,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op - (comparisonPanel?.querySelector('.view-controls')?.clientHeight ?? 0)-12)); panes.forEach(p=>p.style.height=`${height}px`); } - if(v&&panes.length){const size=comparisonSize(v.box.w*vp.comp.width,v.box.h*vp.comp.height,Math.min(...panes.map(p=>p.clientWidth)),Math.min(...panes.map(p=>p.clientHeight)),zoom);root.querySelectorAll('.crop-stage').forEach(el=>{el.style.width=`${size.width}px`;el.style.height=`${size.height}px`;});} + if(v&&panes.length&&!groupOverview){const size=comparisonSize(v.box.w*vp.comp.width,v.box.h*vp.comp.height,Math.min(...panes.map(p=>p.clientWidth)),Math.min(...panes.map(p=>p.clientHeight)),zoom);root.querySelectorAll('.crop-stage').forEach(el=>{el.style.width=`${size.width}px`;el.style.height=`${size.height}px`;});} panes.forEach(p=>{const pannable=p.scrollWidth>p.clientWidth+1||p.scrollHeight>p.clientHeight+1;p.classList.toggle('pannable',pannable);p.style.cursor=expandedComparison?'':'zoom-in';p.setAttribute('role',expandedComparison?'region':'button');p.title=expandedComparison?(pannable?'Move your pointer to pan. You can also scroll, swipe, or use arrow keys.':''):'Click to enlarge comparison';}); if(stage&&frame&&v){const s=stage.clientWidth/(v.box.w*vp.comp.width);frame.style.transform=`scale(${s})`;frame.style.left=`${-v.box.x*vp.comp.width*s}px`;frame.style.top=`${-v.box.y*vp.comp.height*s}px`;} const bounds=workbench.getBoundingClientRect();const region=root.querySelector('.region');const end=root.querySelector('.number'); diff --git a/ui/component-review/styles.ts b/ui/component-review/styles.ts index c3267de84..a8f34ebe2 100644 --- a/ui/component-review/styles.ts +++ b/ui/component-review/styles.ts @@ -102,4 +102,30 @@ export const styles = ` .assembled-review .decisions>button{flex:1;min-width:0} .assembled-review .page-review-status{text-align:left} } + +/* Grouped review is the default; instances are an explicit drill-down. */ +.pin small{font:600 10px/1 var(--font-body, sans-serif);margin-left:3px}.pin:has(small){width:auto;min-width:32px;padding:0 7px} +.instance-region{opacity:.55;border-width:1px;pointer-events:none} +.group-overview>.compare,.group-overview>.view-controls{display:none} +.group-overview{position:relative}.group-overview>.review-peers{padding:0 44px 4px 0;min-height:36px}.group-overview>.compare-toolbar{position:absolute;right:0;top:0;justify-content:flex-end;margin:0} +.group-overview>.compare-toolbar>:not(.comparison-actions),.group-overview .source-link{display:none} +.group-overview .comparison-actions{margin-left:auto} +.review-peers{display:flex;align-items:center;justify-content:space-between;flex-wrap:wrap;gap:8px} +.group-hint{font-size:12px;color:var(--muted)} +.instance-grid{display:grid;gap:0;flex:none;grid-auto-rows:max-content} +.instance-grid-labels{display:grid;grid-template-columns:1fr 1fr;gap:16px;color:var(--muted);font-size:12px;padding:8px 0} +.instance-row{display:block;height:auto;width:100%;text-align:left;padding:12px 0;border:0;border-top:1px solid var(--line);border-radius:0;background:transparent;color:inherit} +.instance-row:hover{background:#eef3f1} +.instance-caption{display:flex;justify-content:space-between;align-items:baseline;gap:12px;font-size:12px;margin-bottom:8px} +.instance-caption>span{color:var(--muted);font-size:11px} +.instance-pair{display:grid;grid-template-columns:1fr 1fr;align-items:center;gap:16px} +.instance-reference{justify-self:center;display:block;position:relative;overflow:hidden;max-height:180px;width:100%;background:var(--comp-background)} +.instance-reference img{position:absolute;max-width:none;height:auto} +.instance-produced{justify-self:center;display:flex;align-items:center;justify-content:center;min-height:36px;max-height:180px;overflow:hidden} +.instance-produced img{display:block;width:100%;height:100%;object-fit:contain} +.instance-row.approved{opacity:.65}.instance-row.feedback .instance-caption>span{color:var(--warn)} +.item strong small{font-size:11px;white-space:nowrap;color:var(--muted)} +#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} `;