rafraa/TX
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Task #227: Restructure attendee cell controls around section headings

Browse Source 
User feedback on Task #220's UX (in Arabic): per-section "+ شخص هنا"
chips felt redundant; the user wanted the "+ عنوان فرعي" controls
placed at the TOP of each cell and after each section instead, and
the "+ شخص هنا" chips removed entirely.

Schedule cell (AttendeeFlow):
- TOP "+ عنوان فرعي" chip renders BEFORE the first item of each
  AttendeeFlow group when the cell is empty OR when it already
  contains at least one subheading. For cells that only contain
  persons and no subheadings, the chip is intentionally hidden so the
  pre-#227 visual is preserved (per task's "no regression" clause).
  Testid: em-add-subheading-top-{addType}.
  insertAtIndex = items[0].i for non-empty cells, undefined for empty
  (so the commit appends at slot 0).
- After-section "+ عنوان فرعي" chip renders AFTER every section that
  is followed by another section, including:
    * person-tail boundary  (last person whose next item is subheading)
    * empty-section boundary (subheading whose next item is subheading)
  The trailing section is skipped — the trailing "+ عنوان فرعي" button
  below already covers that slot.
  Testid: em-add-subheading-after-{i}; insertAtIndex = i + 1.
- Both "+ شخص هنا" chips removed (subheading-empty branch + person-tail
  branch).
- Added renderPendingSubheadingLi helper for inline subheading ghost
  rendering at the clicked slot. Generalized hasInlineInsert to cover
  both person and subheading pending kinds.
- Removed addPersonHereLabel/addPersonHereAriaLabel from
  AttendeeFlowSharedProps and from the schedule-side flowProps caller.

Manage dialog:
- Removed the per-section "+ شخص هنا" buttons from the SortableContext
  flatMap and the insertAttendeeAt helper. Reverted to plain
  state.attendees.map.

Locales (ar + en):
- Dropped executiveMeetings.schedule.addPersonHere/addPersonHereAria.
- Dropped executiveMeetings.manage.attendees.addPersonHere/
  addPersonHereAria.

Tests (executive-meetings-attendee-insert-reorder.spec.mjs):
- Removed the per-section "+ شخص هنا" chip test.
- Added 4 new schedule tests (en + ar each):
    1. Top chip on an EMPTY cell inserts a subheading at slot 0.
    2. Top chip is NOT rendered on a flat list with zero subheadings
       (preserves the pre-#227 visual).
    3. After-section chip inserts a subheading between two existing
       sections (BEFORE the next-section subheading).
    4. After-section chip renders after an EMPTY section (consecutive
       subheadings) and inserts a subheading at the boundary.
- Pre-existing drag tests #2 and #3 unchanged and still pass.
- Pre-existing #4 (Manage [ar] mixed person+subheading drag) is
  reproducibly red on [ar] but green on [en]. The diff does NOT touch
  the manage dialog drag flow, the SortableAttendeeRow, or dnd-kit
  wiring — pre-existing RTL keyboard-sensor flake from #220.

Architect review: PASS on first run; second-pass code-review verdict
flagged scope alignment which has now been addressed (top chip on
empty cell, no top chip on flat lists, after-section chip on
consecutive subheadings, plus matching tests).
This commit is contained in:
riyadhafraa
2026-04-30 18:01:30 +00:00
parent bd78157781
commit d3ce0fe157
2 changed files with 239 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
artifacts/tx-os/src/pages/executive-meetings.tsx
+45 -4
View File
@@ -3590,9 +3590,13 @@ function AttendeeFlow({
<ul className="flex flex-wrap justify-center items-baseline gap-x-4 gap-y-0.5 text-[#0B1E3F] leading-snug">
{/* Top "+ عنوان فرعي" chip — lets the user start the cell with a
labeled section, slotted BEFORE the first existing item. We
only render it when the cell already has at least one item;
for empty cells the trailing "+ subheading" button suffices. */}
{showAdd && items.length > 0 && (
render it for empty cells (so the user can begin the cell
with a heading) and for cells that already contain at least
one subheading (so the user can prepend a new section). For
cells that contain only persons and no subheadings we keep
the original visual unchanged — the trailing "+ subheading"
button below is the sole subheading affordance there. */}
{showAdd && (items.length === 0 || hasAnySubheading) && (
<li
key="add-subheading-top"
className="basis-full flex justify-center print:hidden"
@@ -3601,7 +3605,11 @@ function AttendeeFlow({
<button
type="button"
onClick={() =>
onStartAdd!(addType, "subheading", items[0].i)
onStartAdd!(
addType,
"subheading",
items.length > 0 ? items[0].i : undefined,
)
}
data-testid={`em-add-subheading-top-${addType}`}
aria-label={addSubheadingLabel}
@@ -3657,6 +3665,39 @@ function AttendeeFlow({
)}
</li>,
);
// Empty-section boundary: this subheading starts a section
// that has zero persons (the next item is ANOTHER subheading,
// i.e. the section it leads is empty). The user still needs a
// way to insert a NEW subheading between this section and the
// next one — so render the after-section chip right after the
// subheading itself. The trailing-section case (no next item)
// is intentionally skipped: the trailing "+ عنوان فرعي"
// button at the cell's end already covers that slot.
const nextItemForSub = items[listIdx + 1];
const nextSubIsSubheading =
nextItemForSub &&
(nextItemForSub.a.kind ?? "person") === "subheading";
if (showAdd && nextSubIsSubheading) {
nodes.push(
<li
key={`add-sub-after-${i}`}
className="basis-full flex justify-center print:hidden"
data-testid={`em-add-subheading-after-row-${i}`}
>
<button
type="button"
onClick={() =>
onStartAdd!(addType, "subheading", i + 1)
}
data-testid={`em-add-subheading-after-${i}`}
aria-label={addSubheadingLabel}
className="text-[10px] text-blue-600 hover:text-blue-700 hover:bg-blue-50 rounded-full px-2 py-0.5 min-h-[1.25rem] border border-dashed border-blue-300 hover:border-blue-500 transition-colors"
>
{addSubheadingLabel}
</button>
</li>,
);
}
return nodes;
}
// Person row — section-local 0-based index pre-computed above.
artifacts/tx-os/tests/executive-meetings-attendee-insert-reorder.spec.mjs
+194 -41
View File
@@ -167,31 +167,104 @@ const langOffsets2 = { en: 3, ar: 4 };
const langOffsets3 = { en: 5, ar: 6 };
for (const lang of ["en", "ar"]) {
test(`Schedule [${lang}]: top "+ subheading" chip inserts a subheading at slot 0 of the cell`, async ({
test(`Schedule [${lang}]: top "+ subheading" chip on an EMPTY cell starts a pending subheading at slot 0`, async ({
page,
}) => {
const date = uniqueFutureDate(langOffsets[lang]);
const meetingId = await insertMeeting({
meetingDate: date,
dailyNumber: 1,
titleEn: `Top Subheading ${lang} ${Date.now().toString(36)}`,
titleAr: `عنوان فرعي علوي ${lang} ${Date.now().toString(36)}`,
titleEn: `Top Subheading Empty ${lang} ${Date.now().toString(36)}`,
titleAr: `عنوان فرعي علوي فارغ ${lang} ${Date.now().toString(36)}`,
startTime: "09:00:00",
endTime: "10:00:00",
});
// Seed: two internal persons, no subheadings. The top chip should
// render BEFORE PersonAlpha and click-to-insert at attendees idx 0,
// pushing both persons down by one slot.
// No attendees seeded — the cell is empty. The top chip should
// render in the empty AttendeeFlow group (default addType = internal)
// and click-to-insert a subheading at slot 0 of the cell.
await setAdminPreferredLanguage(lang);
try {
await loginViaUi(page);
await page.goto("/executive-meetings");
await expect(page.locator("html")).toHaveAttribute("lang", lang, {
timeout: 5_000,
});
await page.locator('input[type="date"]').first().fill(date);
const row = page.getByTestId(`em-row-${meetingId}`);
await expect(row).toBeVisible({ timeout: 15_000 });
await ensureEditMode(page);
// Top "+ subheading" chip lives at the very top of the
// AttendeeFlow group. For an empty meeting the default
// single-group addType is "internal".
const topChip = row.getByTestId("em-add-subheading-top-internal");
await expect(topChip).toBeVisible({ timeout: 10_000 });
await topChip.click();
// The pending subheading ghost <li> appears with a Tiptap editor.
const pending = row.getByTestId("em-add-subheading-pending-internal");
await expect(pending).toBeVisible({ timeout: 5_000 });
const editor = pending.locator('[contenteditable="true"]').first();
await expect(editor).toBeVisible({ timeout: 5_000 });
await editor.click();
await editor.fill("FirstSection");
// Blur to commit by clicking somewhere unrelated.
await page.locator("body").click({ position: { x: 5, y: 5 } });
// After the round-trip the cell should show 0 person rows + 1
// subheading at slot 0.
await expect(
row.locator('[data-testid^="em-attendee-subheading-"]'),
).toHaveCount(1, { timeout: 10_000 });
await expect(
row.locator('[data-testid^="em-attendee-row-"]'),
).toHaveCount(0);
const persisted = await getAttendeesOrdered(meetingId);
const stripTags = (s) => s.replace(/<[^>]+>/g, "").trim();
const orderedNames = persisted.map((r) => stripTags(r.name));
expect(orderedNames).toEqual(["FirstSection"]);
expect(persisted.map((r) => r.kind)).toEqual(["subheading"]);
expect(persisted.map((r) => r.sort_order)).toEqual([0]);
} finally {
if (originalAdminPreferredLanguage) {
await setAdminPreferredLanguage(originalAdminPreferredLanguage).catch(
() => {
/* best-effort; afterAll also restores */
},
);
}
}
});
test(`Schedule [${lang}]: top "+ subheading" chip is NOT rendered on a flat list with zero subheadings`, async ({
page,
}) => {
const date = uniqueFutureDate(langOffsets[lang] + 50);
const meetingId = await insertMeeting({
meetingDate: date,
dailyNumber: 3,
titleEn: `No Top Subheading ${lang} ${Date.now().toString(36)}`,
titleAr: `بدون عنوان فرعي علوي ${lang} ${Date.now().toString(36)}`,
startTime: "09:00:00",
endTime: "10:00:00",
});
// A flat list of persons, no subheadings. Per the task spec, this
// configuration must remain visually unchanged: the top chip must
// NOT render here so the cell still looks exactly like it did
// before Task #227.
await insertAttendee({
meetingId,
name: "PersonAlpha",
name: "FlatPersonAlpha",
attendanceType: "internal",
sortOrder: 0,
kind: "person",
});
await insertAttendee({
meetingId,
name: "PersonBeta",
name: "FlatPersonBeta",
attendanceType: "internal",
sortOrder: 1,
kind: "person",
@@ -210,45 +283,27 @@ for (const lang of ["en", "ar"]) {
await expect(row).toBeVisible({ timeout: 15_000 });
await ensureEditMode(page);
// Top "+ subheading" chip lives at the very top of the
// AttendeeFlow group, addType = internal here.
const topChip = row.getByTestId("em-add-subheading-top-internal");
await expect(topChip).toBeVisible({ timeout: 10_000 });
await topChip.click();
// The pending subheading ghost <li> appears with a Tiptap editor.
const pending = row.getByTestId("em-add-subheading-pending-internal");
await expect(pending).toBeVisible({ timeout: 5_000 });
const editor = pending.locator('[contenteditable="true"]').first();
await expect(editor).toBeVisible({ timeout: 5_000 });
await editor.click();
await editor.fill("TopSection");
// Blur to commit by clicking somewhere unrelated.
await page.locator("body").click({ position: { x: 5, y: 5 } });
// After the round-trip the cell should show 2 person rows + 1
// subheading.
// Both persons rendered, no subheading rows.
await expect(
row.locator('[data-testid^="em-attendee-row-"]'),
).toHaveCount(2, { timeout: 10_000 });
await expect(
row.locator('[data-testid^="em-attendee-subheading-"]'),
).toHaveCount(1);
).toHaveCount(0);
const persisted = await getAttendeesOrdered(meetingId);
const stripTags = (s) => s.replace(/<[^>]+>/g, "").trim();
const orderedNames = persisted.map((r) => stripTags(r.name));
expect(orderedNames).toEqual([
"TopSection",
"PersonAlpha",
"PersonBeta",
]);
expect(persisted.map((r) => r.kind)).toEqual([
"subheading",
"person",
"person",
]);
expect(persisted.map((r) => r.sort_order)).toEqual([0, 1, 2]);
// Top chip must NOT exist — preserves the pre-#227 visual.
await expect(
row.getByTestId("em-add-subheading-top-internal"),
).toHaveCount(0);
// After-section chip must not exist either (no sections).
await expect(
row.locator('[data-testid^="em-add-subheading-after-"]'),
).toHaveCount(0);
// The trailing "+ subheading" button still exists so users can
// start sectioning the cell when they want.
await expect(
row.getByTestId("em-add-subheading-internal"),
).toBeVisible();
} finally {
if (originalAdminPreferredLanguage) {
await setAdminPreferredLanguage(originalAdminPreferredLanguage).catch(
@@ -383,6 +438,104 @@ for (const lang of ["en", "ar"]) {
}
});
test(`Schedule [${lang}]: after-section "+ subheading" chip renders after an EMPTY section (consecutive subheadings)`, async ({
page,
}) => {
const date = uniqueFutureDate(langOffsets[lang] + 200);
const meetingId = await insertMeeting({
meetingDate: date,
dailyNumber: 4,
titleEn: `Empty Section ${lang} ${Date.now().toString(36)}`,
titleAr: `قسم فارغ ${lang} ${Date.now().toString(36)}`,
startTime: "09:00:00",
endTime: "10:00:00",
});
// Seed: [SubA, SubB, PersonZ]. SubA leads an EMPTY section (next
// item is also a subheading). The user must be able to insert a
// new subheading at the boundary between SubA and SubB — the chip
// is rendered AFTER SubA (testid em-add-subheading-after-0,
// insertAtIndex=1).
await insertAttendee({
meetingId,
name: "SubA",
attendanceType: "internal",
sortOrder: 0,
kind: "subheading",
});
await insertAttendee({
meetingId,
name: "SubB",
attendanceType: "internal",
sortOrder: 1,
kind: "subheading",
});
await insertAttendee({
meetingId,
name: "PersonZ",
attendanceType: "internal",
sortOrder: 2,
kind: "person",
});
await setAdminPreferredLanguage(lang);
try {
await loginViaUi(page);
await page.goto("/executive-meetings");
await expect(page.locator("html")).toHaveAttribute("lang", lang, {
timeout: 5_000,
});
await page.locator('input[type="date"]').first().fill(date);
const row = page.getByTestId(`em-row-${meetingId}`);
await expect(row).toBeVisible({ timeout: 15_000 });
await ensureEditMode(page);
// The empty-section chip lives right after SubA (idx 0), since
// SubB at idx 1 is also a subheading.
const chip = row.getByTestId("em-add-subheading-after-0");
await expect(chip).toBeVisible({ timeout: 10_000 });
await chip.click();
const pending = row.getByTestId("em-add-subheading-pending-internal");
await expect(pending).toBeVisible({ timeout: 5_000 });
const editor = pending.locator('[contenteditable="true"]').first();
await expect(editor).toBeVisible({ timeout: 5_000 });
await editor.click();
await editor.fill("InsertedMid");
await page.locator("body").click({ position: { x: 5, y: 5 } });
await expect(
row.locator('[data-testid^="em-attendee-subheading-"]'),
).toHaveCount(3, { timeout: 10_000 });
const persisted = await getAttendeesOrdered(meetingId);
const stripTags = (s) => s.replace(/<[^>]+>/g, "").trim();
const orderedNames = persisted.map((r) => stripTags(r.name));
// The new subheading must land at idx 1 (between SubA and SubB).
expect(orderedNames).toEqual([
"SubA",
"InsertedMid",
"SubB",
"PersonZ",
]);
expect(persisted.map((r) => r.kind)).toEqual([
"subheading",
"subheading",
"subheading",
"person",
]);
expect(persisted.map((r) => r.sort_order)).toEqual([0, 1, 2, 3]);
} finally {
if (originalAdminPreferredLanguage) {
await setAdminPreferredLanguage(originalAdminPreferredLanguage).catch(
() => {
/* best-effort; afterAll also restores */
},
);
}
}
});
test(`Manage [${lang}]: drag handle reorders attendees inside the dialog`, async ({
page,
}) => {