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

Task #234: Show one cell-level "+ عنوان فرعي" chip instead of one per group

Browse Source 
Original task: in split-mode meeting cells (cells where 2+ attendance
groups are visible), the trailing "+ عنوان فرعي" chip was being
rendered once per AttendeeGroup. The user reported seeing two chips
(one for the internal group, one for the external group) and asked
"ليش اثنين؟". Per the user's choice in the clarifying interview, we
keep ONE chip at the bottom of the cell, and clicking it adds a new
subheading to the LAST visible attendance group in render order
(virtual → internal → external).

Implementation:
- Added optional `suppressTrailingSubheadingChip` prop to
  `AttendeeFlowSharedProps` (defaults to false, so single-group cells
  are unchanged).
- `AttendeeFlow` skips rendering its trailing per-group "+ عنوان فرعي"
  <li> when the prop is true. The per-group "+" person <li> and all
  after-section chips are unaffected.
- In the `hasSplit` branch of `AttendeesCell`, every per-group
  `AttendeeGroup` is now passed `suppressTrailingSubheadingChip`, and
  one new cell-level chip is rendered after the missing-groups chip
  block with `data-testid="em-add-subheading-cell-${meeting.id}"`. The
  chip is gated by the same `canMutate && !hasAnyPending && startAdd`
  guard as the other add chips. Its onClick computes
  `lastVisibleAddType` at click-time as `external > internal > virtual`
  and calls `startAdd(lastVisibleAddType, "subheading")`.

Tests:
- Updated `Schedule [en|ar]: top "+ subheading" chip is NEVER
  rendered…` so its mixed-meeting cell now asserts the per-group
  `em-add-subheading-{addType}` testids are absent in split mode and
  the new cell-level testid is visible. Per-group "+" person testids
  still prove all three groups mounted.
- Added `Schedule [en|ar]: split-mode cell shows ONE cell-level "+
  subheading" chip and routes to the LAST visible group`. Seeds
  virtual + internal + external attendees, asserts ONE cell-level
  chip + zero per-group subheading chips, clicks it, types a
  subheading, blurs, and verifies the new subheading persists at the
  end of the external group with correct kind / attendance_type /
  sort_order.

Verification:
- tsc clean for executive-meetings.tsx (admin.tsx errors are
  pre-existing and unrelated).
- All 4 new+modified tests pass (en+ar both for the modified top-chip
  test and the new cell-level chip test).
- The 4 unrelated test failures observed in the full run are
  pre-existing flakes (locale state pollution + dnd-kit RTL drag) and
  not caused by this change.
- Architect review: PASS. No critical findings.

Follow-up:
- Task #235 (PROPOSED) covers two test gaps: virtual+internal-only
  fallback locking lastVisibleAddType=internal, and chip-hidden gating
  while a pending ghost is open in another row.
This commit is contained in:
riyadhafraa
2026-04-30 18:44:02 +00:00
parent 40c57b0cb3
commit 27eadae655
2 changed files with 243 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
artifacts/tx-os/src/pages/executive-meetings.tsx
+54 -11
View File
@@ -3220,6 +3220,10 @@ function AttendeesCell({
};
if (hasSplit) {
// Split-mode cells render ONE cell-level "+ عنوان فرعي" chip below
// all groups (see Task #234), so each per-group AttendeeFlow must
// suppress its own trailing subheading chip. Per-group "+" person
// chips and the after-section chips are unaffected.
return (
<div className="space-y-2">
{(virtual.length > 0 || isPendingForType("virtual")) && (
@@ -3233,6 +3237,7 @@ function AttendeesCell({
addType="virtual"
{...flowProps}
isPending={isPendingForType("virtual")}
suppressTrailingSubheadingChip
/>
)}
{(internal.length > 0 || isPendingForType("internal")) && (
@@ -3242,6 +3247,7 @@ function AttendeesCell({
addType="internal"
{...flowProps}
isPending={isPendingForType("internal")}
suppressTrailingSubheadingChip
/>
)}
{(external.length > 0 || isPendingForType("external")) && (
@@ -3251,6 +3257,7 @@ function AttendeesCell({
addType="external"
{...flowProps}
isPending={isPendingForType("external")}
suppressTrailingSubheadingChip
/>
)}
{/* Add chips for groups that don't yet exist for this meeting. */}
@@ -3279,6 +3286,33 @@ function AttendeesCell({
)}
</div>
)}
{/* Cell-level "+ عنوان فرعي" chip (Task #234). One per cell,
not one per group. Clicking it routes the new subheading to
the LAST visible group in render order (virtual → internal
→ external). The chip mirrors the per-group hide rules:
only renders when the user can edit, no add ghost is
already in flight, and a startAdd handler exists. */}
{canMutate && !hasAnyPending && startAdd && (
<div className="flex justify-center print:hidden">
<button
type="button"
onClick={() => {
const lastVisibleAddType: Attendee["attendanceType"] =
external.length > 0
? "external"
: internal.length > 0
? "internal"
: "virtual";
startAdd(lastVisibleAddType, "subheading");
}}
data-testid={`em-add-subheading-cell-${meeting.id}`}
aria-label={t("executiveMeetings.schedule.addSubheading")}
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"
>
{t("executiveMeetings.schedule.addSubheading")}
</button>
</div>
)}
</div>
);
}
@@ -3373,6 +3407,12 @@ type AttendeeFlowSharedProps = {
editSubheadingAriaLabel: string;
newSubheadingAriaLabel: string;
meetingId: number;
// When true, the trailing "+ عنوان فرعي" <li> at the very end of the
// flow is NOT rendered. Used by split-mode cells (≥2 attendance
// groups visible) so the cell shows ONE cell-level "+ عنوان فرعي"
// chip below all groups, instead of one per group. The trailing
// "+" person <li> is unaffected.
suppressTrailingSubheadingChip?: boolean;
};
function AttendeeGroup({
@@ -3417,6 +3457,7 @@ function AttendeeFlow({
newAriaLabel,
editSubheadingAriaLabel,
newSubheadingAriaLabel,
suppressTrailingSubheadingChip = false,
}: { items: AttendeeWithIndex[] } & AttendeeFlowSharedProps) {
const editable = canMutate && !!onSaveAttendeeName;
const showAdd = canMutate && !!onStartAdd && !isPending;
@@ -3784,17 +3825,19 @@ function AttendeeFlow({
+
</button>
</li>
<li className="print:hidden">
<button
type="button"
onClick={() => onStartAdd!(addType, "subheading")}
data-testid={`em-add-subheading-${addType}`}
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>
{!suppressTrailingSubheadingChip && (
<li className="print:hidden">
<button
type="button"
onClick={() => onStartAdd!(addType, "subheading")}
data-testid={`em-add-subheading-${addType}`}
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>
)}
</>
)}
</ul>
artifacts/tx-os/tests/executive-meetings-attendee-insert-reorder.spec.mjs
+189 -7
View File
@@ -167,6 +167,7 @@ test.afterAll(async () => {
const langOffsets = { en: 1, ar: 2 };
const langOffsets2 = { en: 3, ar: 4 };
const langOffsets3 = { en: 5, ar: 6 };
const langOffsets4 = { en: 7, ar: 8 };
for (const lang of ["en", "ar"]) {
test(`Schedule [${lang}]: top "+ subheading" chip is NEVER rendered (empty / person-only / subheading-only / mixed)`, async ({
@@ -267,22 +268,41 @@ for (const lang of ["en", "ar"]) {
// 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}).
// must be absent in every group. We prove all three groups
// mounted by checking each one's per-group trailing "+" person
// button is present (testid em-add-attendee-{addType}). NOTE:
// since Task #234, the trailing "+ subheading" chip is suppressed
// per group in split-mode cells and replaced by ONE cell-level
// chip (testid em-add-subheading-cell-{meetingId}) — so the
// per-group em-add-subheading-{addType} testids must NOT exist
// here, and the cell-level chip MUST be visible.
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.
// Per-group "+" person buttons prove all three groups mounted.
await expect(
mixedRow.getByTestId("em-add-attendee-internal"),
).toBeVisible();
await expect(
mixedRow.getByTestId("em-add-attendee-virtual"),
).toBeVisible();
await expect(
mixedRow.getByTestId("em-add-attendee-external"),
).toBeVisible();
// Per-group trailing "+ subheading" chips are SUPPRESSED in
// split mode. The cell-level chip is the sole subheading
// affordance below all three groups.
await expect(
mixedRow.getByTestId("em-add-subheading-internal"),
).toBeVisible();
).toHaveCount(0);
await expect(
mixedRow.getByTestId("em-add-subheading-virtual"),
).toBeVisible();
).toHaveCount(0);
await expect(
mixedRow.getByTestId("em-add-subheading-external"),
).toHaveCount(0);
await expect(
mixedRow.getByTestId(`em-add-subheading-cell-${meetingMixedId}`),
).toBeVisible();
// Top chip absent in every group.
await expect(
@@ -748,4 +768,166 @@ for (const lang of ["en", "ar"]) {
}
}
});
test(`Schedule [${lang}]: split-mode cell shows ONE cell-level "+ subheading" chip and routes to the LAST visible group`, async ({
page,
}) => {
// Task #234: in a split-mode cell (≥2 attendance groups visible),
// the trailing "+ عنوان فرعي" chip must NOT be rendered per group;
// a single cell-level chip below all groups is rendered instead.
// Clicking it adds the new subheading to the LAST visible group in
// render order (virtual → internal → external). Per-group "+"
// person buttons are unaffected.
//
// hasSplit requires `virtual.length > 0 && (internal || external)`,
// so we seed all three groups. Render order makes EXTERNAL the
// last visible group, and the new subheading must persist at the
// end of the external group's rows.
const date = uniqueFutureDate(langOffsets4[lang]);
const meetingId = await insertMeeting({
meetingDate: date,
dailyNumber: 7,
titleEn: `Cell-Level Subheading ${lang} ${Date.now().toString(36)}`,
titleAr: `عنوان فرعي للخلية ${lang} ${Date.now().toString(36)}`,
startTime: "14:00:00",
endTime: "15:00:00",
});
// Virtual: 1 person (just to satisfy hasSplit — render order
// virtual → internal → external means virtual is first).
await insertAttendee({
meetingId,
name: "VirtualP1",
attendanceType: "virtual",
sortOrder: 0,
kind: "person",
});
// Internal: 2 persons (mirrors the "الحضور الداخلي" block from
// the user's screenshot).
await insertAttendee({
meetingId,
name: "InternalP1",
attendanceType: "internal",
sortOrder: 1,
kind: "person",
});
await insertAttendee({
meetingId,
name: "InternalP2",
attendanceType: "internal",
sortOrder: 2,
kind: "person",
});
// External: 2 persons (mirrors the "الخارجي" block — the LAST
// visible group, so the cell-level chip routes here).
await insertAttendee({
meetingId,
name: "ExternalP1",
attendanceType: "external",
sortOrder: 3,
kind: "person",
});
await insertAttendee({
meetingId,
name: "ExternalP2",
attendanceType: "external",
sortOrder: 4,
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);
// Exactly ONE cell-level chip is rendered.
const cellChip = row.getByTestId(
`em-add-subheading-cell-${meetingId}`,
);
await expect(cellChip).toBeVisible({ timeout: 10_000 });
await expect(
row.locator('[data-testid^="em-add-subheading-cell-"]'),
).toHaveCount(1);
// Per-group trailing "+ subheading" chips are SUPPRESSED in all
// three groups.
await expect(
row.getByTestId("em-add-subheading-virtual"),
).toHaveCount(0);
await expect(
row.getByTestId("em-add-subheading-internal"),
).toHaveCount(0);
await expect(
row.getByTestId("em-add-subheading-external"),
).toHaveCount(0);
// Per-group "+" person buttons remain unchanged — one per group.
await expect(row.getByTestId("em-add-attendee-virtual")).toBeVisible();
await expect(row.getByTestId("em-add-attendee-internal")).toBeVisible();
await expect(row.getByTestId("em-add-attendee-external")).toBeVisible();
// Click the cell-level chip → pending input opens. Because the
// last visible group is external, the pending ghost lives in
// the external group's flow (testid uses the addType suffix).
await cellChip.click();
const pending = row.getByTestId(
"em-add-subheading-pending-external",
);
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("CellChipSection");
await page.locator("body").click({ position: { x: 5, y: 5 } });
// Persisted: the new subheading lands at the end of the external
// group (sort_order 5, after the two external persons).
await expect(
row.locator('[data-testid^="em-attendee-subheading-"]'),
).toHaveCount(1, { timeout: 10_000 });
const persisted = await getAttendeesOrdered(meetingId);
const stripTags = (s) => s.replace(/<[^>]+>/g, "").trim();
expect(persisted.map((r) => stripTags(r.name))).toEqual([
"VirtualP1",
"InternalP1",
"InternalP2",
"ExternalP1",
"ExternalP2",
"CellChipSection",
]);
expect(persisted.map((r) => r.kind)).toEqual([
"person",
"person",
"person",
"person",
"person",
"subheading",
]);
expect(persisted.map((r) => r.attendance_type)).toEqual([
"virtual",
"internal",
"internal",
"external",
"external",
"external",
]);
expect(persisted.map((r) => r.sort_order)).toEqual([0, 1, 2, 3, 4, 5]);
} finally {
if (originalAdminPreferredLanguage) {
await setAdminPreferredLanguage(originalAdminPreferredLanguage).catch(
() => {
/* best-effort; afterAll also restores */
},
);
}
}
});
}