fix(webui): sp9 review round, a step box lists its roles one per line (three at most, then "+N more"), clips its text to the box, and the step panel drops aria-live; tests press Enter on a step and check a four-role step stays inside its box (#1873, lane sp9-workflows)
This commit is contained in:
1 parent
3116f43e65
commit
3597e0daf6
3 files changed
+60
-11
No files matched your search
@@ -30,6 +30,7 @@
|
||||
.wfg-title{font-size:15px;font-weight:600;fill:var(--strong,var(--text))!important}
|
||||
.wfg-roles{font-size:13px;font-family:var(--mono)!important;fill:var(--text)}
|
||||
.wfg-miss{font-size:12px;fill:var(--danger)!important}
|
||||
.wfg-node .wfg-none{fill:var(--muted);font-family:var(--font)}
|
||||
.wfg-edge{fill:none;stroke:var(--muted);stroke-width:1.5}
|
||||
.wfg-arrow{fill:var(--muted)}
|
||||
|
||||
|
||||
@@ -271,9 +271,15 @@
|
||||
const vertical = narrow();
|
||||
const W = vertical ? 300 : 188, GAP = vertical ? 32 : 36, PAD = 14;
|
||||
const per = vertical ? 36 : 21;
|
||||
// Each box is as tall as the tallest step needs: meta, one or two title lines, roles, and the missing-role line if any.
|
||||
const rolesY = st => 46 + (wrap(st.title, per).length - 1) * 19 + 24;
|
||||
const H = Math.max(...steps.map(st => rolesY(st) + (rolesOf(st).some(r => !s.known.has(r)) ? 18 : 0) + 16));
|
||||
// Roles are monospace (13px, about 7.9px a character), so a character budget fits the box; the clip path below catches the rest.
|
||||
const rper = Math.floor((W - 24) / 7.9);
|
||||
// One role a line, at most three lines: with four or more, two roles and "+N more". Every role is in the label and the detail panel.
|
||||
const roleLines = st => { const r = rolesOf(st); return !r.length ? [] : r.length <= 3 ? r : [...r.slice(0, 2), `+${r.length - 2} more`]; };
|
||||
const titleEnd = st => 46 + (wrap(st.title, per).length - 1) * 19;
|
||||
const rolesY = st => titleEnd(st) + 24;
|
||||
const missY = st => rolesY(st) + (Math.max(roleLines(st).length, 1) - 1) * 17 + 18;
|
||||
// Each box is as tall as the tallest step needs: meta, one or two title lines, its role lines, and the missing-role line if any.
|
||||
const H = Math.max(...steps.map(st => (rolesOf(st).some(r => !s.known.has(r)) ? missY(st) : missY(st) - 18) + 16));
|
||||
const pos = i => (vertical ? [PAD, PAD + i * (H + GAP)] : [PAD + i * (W + GAP), PAD]);
|
||||
const width = vertical ? W + 2 * PAD : PAD * 2 + steps.length * W + (steps.length - 1) * GAP;
|
||||
const height = vertical ? PAD * 2 + steps.length * H + (steps.length - 1) * GAP : H + 2 * PAD;
|
||||
@@ -286,22 +292,28 @@
|
||||
const [x, y] = pos(i), n = i + 1, roles = rolesOf(st), missing = roles.filter(r => !s.known.has(r));
|
||||
const lines = wrap(st.title, per), on = sel === n;
|
||||
const label = `Step ${n} of ${steps.length}: ${st.title ?? ''}. ${stateLabel(st)}. Roles ${roles.join(', ')}${missing.length ? `; ${missing.join(', ')} ${MISSING}` : ''}.`;
|
||||
const rl = roleLines(st), more = roles.length > 3;
|
||||
const roleText = rl.length
|
||||
? rl.map((r, j) => `<tspan x="${x + 12}" dy="${j ? 17 : 0}"${more && j === rl.length - 1 ? ' data-wfg-more' : ''}>${esc(more && j === rl.length - 1 ? r : clip(r, rper))}</tspan>`).join('')
|
||||
: '<tspan class="wfg-none">No roles recorded</tspan>';
|
||||
return `<a class="wfg-node${on ? ' is-sel' : ''}" href="${esc(flowHref(org, scope, name, n))}" data-wfg-step="${n}" data-focus-key="${esc(`wfg-step:${n}`)}" aria-label="${esc(label)}"${on ? ' aria-current="step"' : ''}>
|
||||
<title>${esc(st.title)}</title><rect x="${x}" y="${y}" width="${W}" height="${H}" rx="10"/>
|
||||
<title>${esc(`${st.title ?? ''}${roles.length ? ` (${roles.join(', ')})` : ''}`)}</title><rect x="${x}" y="${y}" width="${W}" height="${H}" rx="10"/>
|
||||
<clipPath id="wfg-clip-${n}"><rect x="${x + 2}" y="${y}" width="${W - 10}" height="${H}"/></clipPath>
|
||||
<g clip-path="url(#wfg-clip-${n})">
|
||||
<text class="wfg-meta" x="${x + 12}" y="${y + 22}">Step ${n} · ${esc(stateLabel(st))}</text>
|
||||
<text class="wfg-title" data-wfg-title x="${x + 12}" y="${y + 46}">${lines.map((l, j) => `<tspan x="${x + 12}" dy="${j ? 19 : 0}">${esc(l)}${j < lines.length - 1 ? ' ' : ''}</tspan>`).join('')}</text>
|
||||
<text class="wfg-roles" data-wfg-roles x="${x + 12}" y="${y + rolesY(st)}">${esc(clip(roles.join(', '), per + 4))}</text>
|
||||
${missing.length ? `<text class="wfg-miss" data-wfg-missing x="${x + 12}" y="${y + rolesY(st) + 18}">${esc(clip(`${missing.join(', ')} ${MISSING}`, per + 4))}</text>` : ''}</a>`;
|
||||
<text class="wfg-roles" data-wfg-roles x="${x + 12}" y="${y + rolesY(st)}">${roleText}</text>
|
||||
${missing.length ? `<text class="wfg-miss" data-wfg-missing x="${x + 12}" y="${y + missY(st)}">${esc(clip(`No current role: ${missing.join(", ")}`, rper + 2))}</text>` : ''}</g></a>`;
|
||||
}).join('');
|
||||
return `<svg class="wfg-graph${vertical ? ' is-vertical' : ''}" data-wfg-graph role="group" aria-label="${esc(`${count(steps.length)}, in order`)}" viewBox="0 0 ${width} ${height}" width="${width}" height="${height}" xmlns="http://www.w3.org/2000/svg">
|
||||
<defs><marker id="wfg-arrow" viewBox="0 0 10 10" refX="9" refY="5" markerWidth="8" markerHeight="8" orient="auto"><path class="wfg-arrow" d="M0 0L10 5L0 10z"/></marker></defs>${edges}${nodes}</svg>`;
|
||||
}
|
||||
const stepRef = (org, scope, name, steps, n) => `<a href="${esc(flowHref(org, scope, name, n))}" data-wfg-go="${n}">Step ${n}, ${esc(steps[n - 1].title)}</a>`;
|
||||
function detail(org, s, scope, name, steps, sel) {
|
||||
if (!sel) return `<section class="panel wfg-detail" data-wfg-detail aria-live="polite"><h2>Step details</h2><p class="note">Select a step to see its roles, the state it puts a task in and the step it hands on to.</p></section>`;
|
||||
if (!sel) return `<section class="panel wfg-detail" data-wfg-detail><h2>Step details</h2><p class="note">Select a step to see its roles, the state it puts a task in and the step it hands on to.</p></section>`;
|
||||
const st = steps[sel - 1], roles = rolesOf(st);
|
||||
const roleList = roles.map(r => r === 'human' ? '<code>human</code> (a person)' : s.known.has(r) ? `<code>${esc(r)}</code>` : `<code>${esc(r)}</code> <span class="wf-missing" data-wfg-missing>${MISSING}</span>`).join(', ');
|
||||
return `<section class="panel wfg-detail" data-wfg-detail aria-live="polite"><h2 data-wfg-detail-title>${esc(st.title)}</h2><dl class="wfg-dl">
|
||||
return `<section class="panel wfg-detail" data-wfg-detail><h2 data-wfg-detail-title>${esc(st.title)}</h2><dl class="wfg-dl">
|
||||
<dt>Position</dt><dd data-wfg-field="position">Step ${sel} of ${steps.length}</dd>
|
||||
<dt>Roles</dt><dd data-wfg-field="roles">${roleList || '<span class="muted">None recorded</span>'}</dd>
|
||||
<dt>Puts the task in</dt><dd data-wfg-field="state">${esc(stateLabel(st))}</dd>
|
||||
|
||||
@@ -109,7 +109,8 @@ test('the graph draws one box per step in order with its roles and one arrowed e
|
||||
assert.equal(await b.evaluate(`document.querySelector('[data-wfg-graph]').tagName.toLowerCase()`), 'svg');
|
||||
assert.deepEqual(await b.evaluate(`${boxes}.map(g=>g.dataset.wfgStep)`), ['1', '2', '3', '4']);
|
||||
assert.deepEqual(await b.evaluate(`${boxes}.map(g=>g.querySelector('[data-wfg-title]').textContent)`), ['Write the task', 'Implement the change', 'Review the change', 'Approve the merge']);
|
||||
assert.deepEqual(await b.evaluate(`${boxes}.map(g=>g.querySelector('[data-wfg-roles]').textContent)`), ['pm', 'coder', 'reviewer', 'human, merge-gate']);
|
||||
// One role per line.
|
||||
assert.deepEqual(await b.evaluate(`${boxes}.map(g=>[...g.querySelectorAll('[data-wfg-roles] tspan')].map(t=>t.textContent))`), [['pm'], ['coder'], ['reviewer'], ['human', 'merge-gate']]);
|
||||
// A role that resolves to no definition or template (and is not human) is marked on its box.
|
||||
assert.deepEqual(await b.evaluate(`${boxes}.map(g=>g.querySelectorAll('[data-wfg-missing]').length)`), [0, 0, 0, 1]);
|
||||
// At 1440 the steps run left to right.
|
||||
@@ -152,8 +153,15 @@ test('selecting a step shows what the data holds for it and survives a repaint',
|
||||
assert.equal(await field('next'), 'Nothing: this is the last step');
|
||||
assert.match(await field('roles'), /human \(a person\)/);
|
||||
assert.match(await field('roles'), /merge-gate names no current role/);
|
||||
// Keyboard: a step box is a link that takes focus and opens on Enter.
|
||||
assert.equal(await b.evaluate(`document.querySelector('[data-wfg-step="1"]').closest('a,[tabindex]') !== null`), true);
|
||||
// Keyboard: a step box takes focus and opens on Enter, and keeps focus through the repaint.
|
||||
await b.evaluate(`document.querySelector('[data-wfg-step="1"]').focus();true`);
|
||||
assert.equal(await b.evaluate(`document.activeElement.dataset.focusKey`), 'wfg-step:1');
|
||||
await b.key('Enter');
|
||||
await wait(`document.querySelector('[data-wfg-detail] [data-wfg-detail-title]')?.textContent === 'Write the task' && document.activeElement?.dataset.focusKey === 'wfg-step:1'`, 'Enter opens step 1 and keeps focus');
|
||||
assert.equal(await b.evaluate(`location.hash`), '#/orgs/acme/workflows/change?step=1');
|
||||
await b.evaluate(`document.querySelector('[data-wfg-step="4"]').focus();true`);
|
||||
await b.key('Enter');
|
||||
await wait(`document.querySelector('[data-wfg-detail] [data-wfg-detail-title]')?.textContent === 'Approve the merge'`, 'Enter opens step 4');
|
||||
// A repaint (the poll) keeps the selection.
|
||||
const before = reads.length;
|
||||
await b.evaluate(`document.dispatchEvent(new CustomEvent('mosaic-workspace-render'));true`);
|
||||
@@ -168,6 +176,34 @@ test('selecting a step shows what the data holds for it and survives a repaint',
|
||||
await wait(`/No workflow named nope/.test(document.querySelector('#bus-view').textContent)`, 'unknown workflow');
|
||||
});
|
||||
|
||||
test('a step with more roles than fit shows three lines, the last counting the rest, all inside its box', { timeout: 60000 }, async t => {
|
||||
// The business-management pack's first step names four long roles.
|
||||
const roles = ['chief-executive-officer', 'chief-financial-officer', 'chief-information-officer', 'chief-information-security-officer'];
|
||||
DOCS.acme.workflows.brief = { steps: [{ title: 'Draft the brief', state: 'todo', roles }, { title: 'WWWWWWWWWWWWWWWWWWWWWWWW WWWWWWWWWWWWWWWWWWWWWWWW WWWWWWWWWWWWWWWWWWWW', state: 'done', roles: ['human'] }] };
|
||||
t.after(() => { delete DOCS.acme.workflows.brief; });
|
||||
for (const viewport of [[1440, 900], [390, 844]]) {
|
||||
await t.test(String(viewport[0]), async t2 => {
|
||||
const { b, wait } = await setup(t2, { path: '#/orgs/acme/workflows/brief', viewport });
|
||||
await wait(`${boxes}.length === 2`, 'two boxes');
|
||||
const one = `document.querySelector('[data-wfg-step="1"]')`;
|
||||
// A role longer than the box is shortened with an ellipsis (the full name is in the box's title and the step panel).
|
||||
const lines = await b.evaluate(`[...${one}.querySelectorAll('[data-wfg-roles] tspan')].map(t=>t.textContent)`);
|
||||
assert.equal(lines.length, 3);
|
||||
assert.equal(lines[2], '+2 more');
|
||||
lines.slice(0, 2).forEach((line, i) => assert.ok(line === roles[i] || (line.endsWith('…') && roles[i].startsWith(line.slice(0, -1))), line));
|
||||
if (viewport[0] === 390) assert.deepEqual(lines.slice(0, 2), roles.slice(0, 2), 'the 390 column is wide enough for the whole name');
|
||||
assert.equal(await b.evaluate(`${one}.querySelector('[data-wfg-roles] [data-wfg-more]').textContent`), '+2 more');
|
||||
// The box names every role for a pointer or a screen reader.
|
||||
const named = await b.evaluate(`${one}.querySelector('title').textContent + ' ' + (${one}.getAttribute('aria-label') ?? '')`);
|
||||
for (const r of roles) assert.ok(named.includes(r), r);
|
||||
// Every text line ends inside its own box.
|
||||
const out = await b.evaluate(`${boxes}.flatMap(g=>{const r=g.querySelector('rect').getBoundingClientRect();return [...g.querySelectorAll('text tspan, text:not(:has(tspan))')].filter(t=>{const q=t.getBoundingClientRect();const c=g.querySelector('clipPath rect')?.getBoundingClientRect()??r;return Math.min(q.right,c.right)>r.right+0.5||Math.min(q.bottom,c.bottom)>r.bottom+0.5}).map(t=>t.textContent)})`);
|
||||
assert.deepEqual(out, []);
|
||||
assert.equal(await b.evaluate(`document.documentElement.scrollWidth<=innerWidth`), true);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test('an organization with no workflows says so on its page and its card, and links to apply a pack', { timeout: 60000 }, async t => {
|
||||
const { b, wait } = await setup(t, { path: '#/orgs/bare/workflows' });
|
||||
await wait(`document.querySelector('[data-wfg-list] [data-wfg-none]')`, 'none note');
|
||||
|
||||
Reference in new issue
Block a user