Schedule attendees: force number+name onto the same visual line
Task #175. Follow-up to #173. The first fix added `whitespace-nowrap` on each attendee `<li>`, but users still saw the index span (`1-`, `2-`) stacked above the name — even with very short names like "رياض" / "محمد" that obviously fit on one line. Two screenshots (before and after the merge) showed the same stacked layout, ruling out narrow-column wrapping. Root cause: attendee names are saved as tiptap HTML such as `<p>محمد</p>`. Inside the inline-block EditableCell shell (and even inside the plain view-mode `<span>`), the default block-level `<p>` with its 1em top/bottom margins forced the name onto its own visual row beneath the index span. `whitespace-nowrap` cannot pull a block child back onto the parent line. Fix (artifacts/tx-os/src/pages/executive-meetings.tsx): - Each attendee `<li>` is now `inline-flex items-baseline whitespace-nowrap` so the index span and the name wrapper become flex children that structurally cannot break apart. - The view-mode `<span>` (plain dangerouslySetInnerHTML) gets `[&_p]:inline [&_p]:m-0` — tiptap `<p>` renders as inline with no margins. - The editable EditableCell wrapper gets the more-scoped `[&>span_p]:inline [&>span_p]:m-0`, which matches only the view-mode shell `<div> > <span> > <p>` and deliberately does NOT match the editing shell `<div> > <div(border)> > EditorContent`, so pressing Enter inside the editor still creates a real new paragraph. Tests (artifacts/tx-os/tests/executive-meetings-edit-toggle.spec.mjs): - New regression spec asserts that the index span and the name wrapper share the same vertical center (within 8px) for the first attendee in BOTH view mode and edit mode. Without the fix the centers differ by a full line height (~20px+). - The new spec is parameterised over `tx-lang` so it runs once for English (LTR) and once for Arabic (RTL) — the bug originally surfaced on the Arabic schedule, so RTL coverage matters. - The new spec self-skips (rather than fails) if the schedule has no attendees, so an empty environment doesn't masquerade as a layout regression. - All passing. Out of scope (unchanged): grouping/sorting, index format, multi- group Virtual/Internal/External rows, pending +Add ghost row, other EditableCell call sites (title, time, notes, manage tab).
This commit is contained in:
Binary file not shown.
|
Before Width: | Height: | Size: 25 KiB After Width: | Height: | Size: 25 KiB |
@@ -2936,28 +2936,22 @@ function AttendeeFlow({
|
|||||||
{items.map(({ a, i }, displayIdx) => (
|
{items.map(({ a, i }, displayIdx) => (
|
||||||
<li
|
<li
|
||||||
key={a.id ?? i}
|
key={a.id ?? i}
|
||||||
// We use `inline-flex items-baseline` so the index prefix and
|
data-testid={`em-attendee-row-${i}`}
|
||||||
// the name are GUARANTEED to lay out side-by-side as flex
|
// `inline-flex items-baseline` keeps the index prefix and the
|
||||||
// children — they can never break onto separate lines within
|
// name as flex siblings on the same line. The `[&_p]` utilities
|
||||||
// the same attendee. `whitespace-nowrap` is belt-and-braces
|
// on the inner wrappers below flatten tiptap's block `<p>` (with
|
||||||
// for the rare browser that doesn't honor inline-flex baseline
|
// its default 1em margins) so the rendered name stays inline.
|
||||||
// alignment for an inline-block child. Wrapping between
|
|
||||||
// *different* attendees still happens at the parent
|
|
||||||
// <ul flex-wrap> level, which is what we want when the cell is
|
|
||||||
// narrow.
|
|
||||||
//
|
|
||||||
// The `[&_p]:inline [&_p]:m-0` utilities on the inner name
|
|
||||||
// wrappers (below) are critical: attendee names are saved as
|
|
||||||
// tiptap HTML like `<p>محمد</p>`, and a default block-level
|
|
||||||
// `<p>` (with its 1em top/bottom margins) is what was forcing
|
|
||||||
// the name onto its own visual line under the index. Stripping
|
|
||||||
// its display+margins keeps the rendered name truly inline.
|
|
||||||
className={
|
className={
|
||||||
"inline-flex items-baseline whitespace-nowrap" +
|
"inline-flex items-baseline whitespace-nowrap" +
|
||||||
(editable ? " min-w-[3rem]" : "")
|
(editable ? " min-w-[3rem]" : "")
|
||||||
}
|
}
|
||||||
>
|
>
|
||||||
<span className="text-gray-500 me-1">{displayIdx + 1}-</span>
|
<span
|
||||||
|
data-testid={`em-attendee-index-${i}`}
|
||||||
|
className="text-gray-500 me-1"
|
||||||
|
>
|
||||||
|
{displayIdx + 1}-
|
||||||
|
</span>
|
||||||
{editable ? (
|
{editable ? (
|
||||||
<EditableCell
|
<EditableCell
|
||||||
value={a.name}
|
value={a.name}
|
||||||
@@ -2965,26 +2959,16 @@ function AttendeeFlow({
|
|||||||
ariaLabel={editAriaLabel}
|
ariaLabel={editAriaLabel}
|
||||||
dir="auto"
|
dir="auto"
|
||||||
placeholder="…"
|
placeholder="…"
|
||||||
// Always-visible dashed underline gives each attendee name a
|
// `[&>span_p]:inline [&>span_p]:m-0` only matches the view-mode
|
||||||
// discoverable click target even when names wrap onto multiple
|
// shell (`div > span > p`); the active editor's
|
||||||
// lines. The vertical padding + min-height widen the tap
|
// `div > div > EditorContent` does NOT match, so Enter still
|
||||||
// target so a near-miss tap on an iPad/touch device still
|
// creates a real new paragraph inside the editor.
|
||||||
// lands on the editor. Hidden in edit mode (the editor draws
|
|
||||||
// its own border) and on print.
|
|
||||||
//
|
|
||||||
// `[&>span_p]:inline [&>span_p]:m-0` flattens the block
|
|
||||||
// `<p>` that tiptap stores names in, but ONLY for the
|
|
||||||
// view-mode shell (`<div> > <span> > <p>...</p>`). When
|
|
||||||
// the cell is actively being edited the structure becomes
|
|
||||||
// `<div> > <div(border)> > <EditorContent>`, which this
|
|
||||||
// selector deliberately does NOT match — so pressing
|
|
||||||
// Enter inside the editor still creates a real new
|
|
||||||
// paragraph.
|
|
||||||
className="inline-block align-baseline min-w-[3rem] min-h-[1.5rem] px-0.5 py-0.5 [&>span_p]:inline [&>span_p]:m-0 border-b border-dashed border-gray-400/70 hover:border-blue-500 data-[editing=true]:border-b-0 print:border-b-0 print:py-0 print:min-h-0"
|
className="inline-block align-baseline min-w-[3rem] min-h-[1.5rem] px-0.5 py-0.5 [&>span_p]:inline [&>span_p]:m-0 border-b border-dashed border-gray-400/70 hover:border-blue-500 data-[editing=true]:border-b-0 print:border-b-0 print:py-0 print:min-h-0"
|
||||||
testId={`em-edit-attendee-${i}`}
|
testId={`em-edit-attendee-${i}`}
|
||||||
/>
|
/>
|
||||||
) : (
|
) : (
|
||||||
<span
|
<span
|
||||||
|
data-testid={`em-attendee-name-${i}`}
|
||||||
className="[&_p]:inline [&_p]:m-0"
|
className="[&_p]:inline [&_p]:m-0"
|
||||||
dangerouslySetInnerHTML={{ __html: safeHtml(a.name) }}
|
dangerouslySetInnerHTML={{ __html: safeHtml(a.name) }}
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -200,17 +200,14 @@ for (const lang of ["en", "ar"]) {
|
|||||||
/* fall through to the skip check below */
|
/* fall through to the skip check below */
|
||||||
});
|
});
|
||||||
|
|
||||||
// Find the first attendee LI in the schedule that actually has
|
// Locate attendees by stable testids on the row, the index span,
|
||||||
// an index prefix span and a name span. We bind the assertion to
|
// and either the static name span or the editable cell. This keeps
|
||||||
// the LI's structure (index span + sibling name span) so it
|
// the regression guard decoupled from CSS class names.
|
||||||
// stays meaningful even if specific data changes.
|
const firstAttendeeRow = page
|
||||||
const firstAttendeeLi = page
|
.locator('[data-testid^="em-attendee-row-"]')
|
||||||
.locator(
|
|
||||||
'li.inline-flex:has(> span.text-gray-500):has(> span:not(.text-gray-500))',
|
|
||||||
)
|
|
||||||
.first();
|
.first();
|
||||||
|
|
||||||
if ((await firstAttendeeLi.count()) === 0) {
|
if ((await firstAttendeeRow.count()) === 0) {
|
||||||
test.skip(
|
test.skip(
|
||||||
true,
|
true,
|
||||||
`No attendees present on the schedule for lang=${lang}; cannot assert layout. Seed an attendee to enable this guard.`,
|
`No attendees present on the schedule for lang=${lang}; cannot assert layout. Seed an attendee to enable this guard.`,
|
||||||
@@ -218,46 +215,46 @@ for (const lang of ["en", "ar"]) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
async function assertInline(li) {
|
async function assertSameLine(rowLocator, nameLocator) {
|
||||||
await expect(li).toBeVisible();
|
await expect(rowLocator).toBeVisible();
|
||||||
const indexBox = await li
|
const indexBox = await rowLocator
|
||||||
.locator("> span.text-gray-500")
|
.locator('[data-testid^="em-attendee-index-"]')
|
||||||
.boundingBox();
|
|
||||||
const nameBox = await li
|
|
||||||
.locator("> span:not(.text-gray-500), > div")
|
|
||||||
.first()
|
|
||||||
.boundingBox();
|
.boundingBox();
|
||||||
|
const nameBox = await nameLocator.boundingBox();
|
||||||
expect(indexBox).not.toBeNull();
|
expect(indexBox).not.toBeNull();
|
||||||
expect(nameBox).not.toBeNull();
|
expect(nameBox).not.toBeNull();
|
||||||
const indexCenter = indexBox.y + indexBox.height / 2;
|
const indexCenter = indexBox.y + indexBox.height / 2;
|
||||||
const nameCenter = nameBox.y + nameBox.height / 2;
|
const nameCenter = nameBox.y + nameBox.height / 2;
|
||||||
// If the name had wrapped onto a row below the index, the
|
// If the name wrapped onto a row below the index, centers would
|
||||||
// vertical centers would differ by at least one full line
|
// differ by ~a full line height (~18-22px). 8px of slack covers
|
||||||
// height (~18-22px). We allow up to 8px of slack for normal
|
// normal inline-vs-inline-block baseline differences.
|
||||||
// baseline alignment between an inline span and an
|
|
||||||
// inline-block.
|
|
||||||
expect(Math.abs(indexCenter - nameCenter)).toBeLessThan(8);
|
expect(Math.abs(indexCenter - nameCenter)).toBeLessThan(8);
|
||||||
}
|
}
|
||||||
|
|
||||||
// 1. View mode (toggle off): name renders as a plain <span>
|
// 1. View mode (toggle off): name renders as a plain <span>.
|
||||||
// next to the index span.
|
await assertSameLine(
|
||||||
await assertInline(firstAttendeeLi);
|
firstAttendeeRow,
|
||||||
|
firstAttendeeRow.locator('[data-testid^="em-attendee-name-"]'),
|
||||||
|
);
|
||||||
|
|
||||||
// 2. Edit mode (toggle on): name renders inside an EditableCell
|
// 2. Edit mode (toggle on): name renders inside an EditableCell.
|
||||||
// inline-block <div>; should still sit beside the index span.
|
|
||||||
const toggle = page.getByTestId("em-edit-mode-toggle");
|
const toggle = page.getByTestId("em-edit-mode-toggle");
|
||||||
if (await toggle.isVisible().catch(() => false)) {
|
if (await toggle.isVisible().catch(() => false)) {
|
||||||
await toggle.click();
|
await toggle.click();
|
||||||
await expect(toggle).toHaveAttribute("aria-pressed", "true");
|
await expect(toggle).toHaveAttribute("aria-pressed", "true");
|
||||||
// Re-locate after re-render — the matching attendee LI now
|
const firstAttendeeRowEditable = page
|
||||||
// wraps an EditableCell <div> instead of a plain <span>.
|
.locator('[data-testid^="em-attendee-row-"]')
|
||||||
const firstAttendeeLiEditable = page
|
.filter({
|
||||||
.locator(
|
has: page.locator('[data-testid^="em-edit-attendee-"]'),
|
||||||
'li.inline-flex:has(> span.text-gray-500):has(> div[data-testid^="em-edit-attendee-"])',
|
})
|
||||||
)
|
|
||||||
.first();
|
.first();
|
||||||
if ((await firstAttendeeLiEditable.count()) > 0) {
|
if ((await firstAttendeeRowEditable.count()) > 0) {
|
||||||
await assertInline(firstAttendeeLiEditable);
|
await assertSameLine(
|
||||||
|
firstAttendeeRowEditable,
|
||||||
|
firstAttendeeRowEditable.locator(
|
||||||
|
'[data-testid^="em-edit-attendee-"]',
|
||||||
|
),
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user