Task #230: Remove redundant top "+ subheading" chip in attendee cells

User feedback on Task #227: the top "+ عنوان فرعي" chip is visually
redundant in every state (duplicates the trailing chip on empty /
single-section cells, and feels like noise above the first heading
once a heading exists). The trailing "+ عنوان فرعي" + after-section
chips already cover every insertion point the user actually needs.

Schedule cell (AttendeeFlow):
- Removed the entire top "+ عنوان فرعي" chip JSX block (the gate
  `items.length === 0 || hasAnySubheading`, the testid
  `em-add-subheading-top-{addType}`, and surrounding comment).
- Updated the surviving comments on `hasInlineInsert` and
  `renderPendingSubheadingLi` so they no longer mention the now-gone
  top chip — they reference only the after-section chip flow.
- `hasAnySubheading` is kept (still used by person-row numbering and
  pending-person-ghost numbering). Per-section after-section chips
  (subheading- and person-branch) and the trailing
  "+ عنوان فرعي" / "+" buttons are unchanged.

Tests (executive-meetings-attendee-insert-reorder.spec.mjs):
- Removed the two top-chip tests:
    1. "top chip on EMPTY cell inserts a subheading at slot 0"
    2. "top chip is NOT rendered on a flat list with zero subheadings"
- Replaced them with a single per-locale test:
    "top \"+ subheading\" chip is NEVER rendered (empty /
     person-only / subheading-only / mixed)".
  It seeds two meetings: an empty one (covers EMPTY state) and a
  three-section meeting that spreads attendees across the
  internal/virtual/external attendance-type groups so each
  AttendeeFlow renders in a different state (person-only,
  subheading-only, mixed). Asserts `em-add-subheading-top-*` has
  count 0 across the row, plus a sanity check on the mixed-row
  attendee count so the 0-count is meaningful.
- After-section tests (between two sections / after empty section)
  and manage drag tests are unchanged.
- Updated the file's top-of-file comment to mention #230 alongside
  #227.

Verification:
- 6 schedule tests in executive-meetings-attendee-insert-reorder
  PASS in en + ar (38.7s).
- 6 regression tests in executive-meetings-attendee-subheadings
  PASS in en + ar (34.4s).
- `tsc --noEmit` reports zero errors in executive-meetings.tsx.
- Pre-existing dnd-kit RTL flake (Manage [ar/en]: drag handle
  reorders attendees inside the dialog) bounces locales between
  runs; unrelated to this diff (no changes to manage dialog drag
  flow, SortableAttendeeRow, or dnd-kit wiring).

No deviations from the task spec.
This commit is contained in:
riyadhafraa
2026-04-30 18:19:13 +00:00
parent 6deed137cf
commit aaeb0a48a0
2 changed files with 112 additions and 153 deletions
@@ -1,20 +1,21 @@
import { test, expect } from "@playwright/test";
import pg from "pg";
// E2E coverage for Task #227 — "Restructure attendee cell controls
// around section headings" inside executive-meetings.
// E2E coverage for Tasks #227 and #230 — "Restructure attendee cell
// controls around section headings" inside executive-meetings.
//
// What we verify:
// 1. Top "+ عنوان فرعي" chip on the schedule grid: the chip is
// rendered BEFORE the first attendee of each AttendeeFlow group.
// Clicking it opens a pending subheading ghost at slot 0 (or the
// group's first global index) and committing the typed text
// persists the new subheading at the very top of the cell.
// 1. The top "+ عنوان فرعي" chip is NEVER rendered, regardless of
// cell state (empty / person-only / subheading-only / mixed).
// The trailing "+ عنوان فرعي" button at the bottom of the cell is
// the sole "start a heading" affordance, plus the after-section
// chips between consecutive sections.
// 2. After-section "+ عنوان فرعي" chip: rendered after the last
// person of each section that is followed by ANOTHER section
// (i.e. a subheading comes next). Clicking it inserts a new
// subheading at the boundary, BEFORE the existing next-section
// subheading.
// subheading. Also rendered after a subheading whose next item is
// another subheading (empty-section boundary).
// 3. Drag-and-drop reordering inside the manage dialog: picking up a
// row by its grip handle and moving it down with the keyboard
// reorders the attendee array, and saving persists the new order.
@@ -110,7 +111,7 @@ async function setAdminPreferredLanguage(lang) {
return rows[0]?.preferred_language;
}
// Toggle the global edit-mode toggle so the top / after-section
// Toggle the global edit-mode toggle so the after-section
// "+ عنوان فرعي" chips and inline editing affordances render. The
// toggle lives in the schedule toolbar with
// data-testid="em-edit-mode-toggle".
@@ -168,108 +169,78 @@ const langOffsets2 = { en: 3, ar: 4 };
const langOffsets3 = { en: 5, ar: 6 };
for (const lang of ["en", "ar"]) {
test(`Schedule [${lang}]: top "+ subheading" chip on an EMPTY cell starts a pending subheading at slot 0`, async ({
test(`Schedule [${lang}]: top "+ subheading" chip is NEVER rendered (empty / person-only / subheading-only / mixed)`, async ({
page,
}) => {
const date = uniqueFutureDate(langOffsets[lang]);
const meetingId = await insertMeeting({
meetingDate: date,
// Task #230: the top chip is gone in every state. We seed two
// meetings that, between them, exercise all four cell states:
// meetingEmptyId: a brand-new meeting with zero attendees ⇒
// the cell is EMPTY.
// meetingMixedId: three sections covering person-only,
// subheading-only, and mixed (subheading +
// persons) by spreading attendees across the
// three attendance-type groups (internal,
// virtual, external) which each render their own
// AttendeeFlow.
// For each addType we assert the top chip testid is absent. The
// trailing "+ subheading" button stays as the cell-level fallback.
const dateEmpty = uniqueFutureDate(langOffsets[lang]);
const meetingEmptyId = await insertMeeting({
meetingDate: dateEmpty,
dailyNumber: 1,
titleEn: `Top Subheading Empty ${lang} ${Date.now().toString(36)}`,
titleAr: `عنوان فرعي علوي فارغ ${lang} ${Date.now().toString(36)}`,
titleEn: `No Top Empty ${lang} ${Date.now().toString(36)}`,
titleAr: `بدون علوي فارغ ${lang} ${Date.now().toString(36)}`,
startTime: "09:00:00",
endTime: "10:00:00",
});
// 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)}`,
const dateMixed = uniqueFutureDate(langOffsets[lang] + 50);
const meetingMixedId = await insertMeeting({
meetingDate: dateMixed,
dailyNumber: 2,
titleEn: `No Top Mixed ${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.
// internal: person-only (two persons, zero subheadings).
await insertAttendee({
meetingId,
name: "FlatPersonAlpha",
meetingId: meetingMixedId,
name: "InternalPerson1",
attendanceType: "internal",
sortOrder: 0,
kind: "person",
});
await insertAttendee({
meetingId,
name: "FlatPersonBeta",
meetingId: meetingMixedId,
name: "InternalPerson2",
attendanceType: "internal",
sortOrder: 1,
kind: "person",
});
// virtual: subheading-only (one subheading, zero persons).
await insertAttendee({
meetingId: meetingMixedId,
name: "VirtualSoloHeader",
attendanceType: "virtual",
sortOrder: 2,
kind: "subheading",
});
// external: mixed (subheading + person).
await insertAttendee({
meetingId: meetingMixedId,
name: "ExternalHeader",
attendanceType: "external",
sortOrder: 3,
kind: "subheading",
});
await insertAttendee({
meetingId: meetingMixedId,
name: "ExternalPerson",
attendanceType: "external",
sortOrder: 4,
kind: "person",
});
await setAdminPreferredLanguage(lang);
try {
@@ -278,33 +249,53 @@ for (const lang of ["en", "ar"]) {
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 });
// EMPTY cell ⇒ default single addType group "internal" renders
// with no items.
await page.locator('input[type="date"]').first().fill(dateEmpty);
const emptyRow = page.getByTestId(`em-row-${meetingEmptyId}`);
await expect(emptyRow).toBeVisible({ timeout: 15_000 });
await ensureEditMode(page);
// 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-"]'),
emptyRow.locator('[data-testid^="em-add-subheading-top-"]'),
).toHaveCount(0);
// Top chip must NOT exist — preserves the pre-#227 visual.
// The trailing "+ subheading" button is still present so the
// user can begin sectioning the cell.
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"),
emptyRow.getByTestId("em-add-subheading-internal"),
).toBeVisible();
// MIXED-meeting cell ⇒ three AttendeeFlow groups (internal /
// virtual / external) each in a different state. The top chip
// must be absent in every group, and we directly verify each
// group mounted by checking its own trailing "+ subheading"
// button is present (testid em-add-subheading-{addType}).
await page.locator('input[type="date"]').first().fill(dateMixed);
const mixedRow = page.getByTestId(`em-row-${meetingMixedId}`);
await expect(mixedRow).toBeVisible({ timeout: 15_000 });
// Each addType group renders its own trailing "+ subheading"
// button — proving all three AttendeeFlow groups are mounted.
await expect(
mixedRow.getByTestId("em-add-subheading-internal"),
).toBeVisible();
await expect(
mixedRow.getByTestId("em-add-subheading-virtual"),
).toBeVisible();
await expect(
mixedRow.getByTestId("em-add-subheading-external"),
).toBeVisible();
// Top chip absent in every group.
await expect(
mixedRow.locator('[data-testid^="em-add-subheading-top-"]'),
).toHaveCount(0);
// Sanity counts (defence-in-depth so the assertion above isn't
// trivially passing because the groups silently rendered empty).
await expect(
mixedRow.locator('[data-testid^="em-attendee-row-"]'),
).toHaveCount(3);
await expect(
mixedRow.locator('[data-testid^="em-attendee-subheading-"]'),
).toHaveCount(2);
} finally {
if (originalAdminPreferredLanguage) {
await setAdminPreferredLanguage(originalAdminPreferredLanguage).catch(