Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions docs/proposals/mockups/proposal-h/SPEC-display-left-rail.md

Large diffs are not rendered by default.

21 changes: 20 additions & 1 deletion frontend/src/components/AutofillCollapse.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ interface AutofillCollapseProps {
* (every pre-existing caller) renders exactly as before, just gaining the always-safe
* `aria-expanded` below. */
id?: string;
/** Additive, optional (CSS-fidelity source-map pass, SPEC-display-left-rail.md §2/§9 -
* "AutofillCollapse header in rail: Superhero's stock `.card-header` `0.5rem 1rem` (8/16) ->
* rail-scoped `padding:7px 10px`"). Component-scoped replacement for what used to be a
* `RailRoot`-level `.card-header{padding:7px 10px}` descendant-selector override living two
* files away from the `Card.Header` it targeted (DisplayPage.tsx's own RailRoot, clobbering
* Bootstrap's global `card.scss` rule by selector specificity, not by scope) - see that
* commit's own note in SPEC-display-left-rail.md's "Source map addendum" for why that pattern
* is the recurrence signature this prop retires. `undefined` (every pre-existing caller, and
* every non-rail caller of this shared component - CardDetailedViewBody/PDFGenerator/
* JumpToVersion/CardResultSet/GridSelectorFilters) renders with Bootstrap's own stock padding,
* byte-for-byte unchanged. */
headerPadding?: string;
}

/**
Expand All @@ -42,6 +54,7 @@ export function AutofillCollapse({
sticky = false,
pad = 0,
id,
headerPadding,
}: AutofillCollapseProps) {
return (
<>
Expand All @@ -52,9 +65,15 @@ export function AutofillCollapse({
aria-expanded={expanded}
aria-controls={id != null ? `${id}-body` : undefined}
style={{
backgroundColor: "#4E5D6B",
// CSS-fidelity source-map pass (SPEC-display-left-rail.md §0) - was "#4E5D6B" (a
// hand-typed literal one hex digit off the real theme token in the blue channel,
// 0x6B vs 0x6C - imperceptible but never actually sourced from the theme). Corrected
// to the exact `$secondary`/`$card-bg` value (#4e5d6c) SPEC-display-left-rail.md §0
// documents.
backgroundColor: "#4e5d6c",
zIndex: zIndex + 1,
cursor: "pointer",
...(headerPadding != null ? { padding: headerPadding } : {}),
}}
>
<Stack direction="horizontal" gap={2} className="d-flex px-0">
Expand Down
24 changes: 15 additions & 9 deletions frontend/src/features/display/DisplayPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,12 @@ const RailSection = ({
expanded={expandedSections[sectionKey]}
onClick={() => onToggle(sectionKey)}
pad={2}
// CSS-fidelity source-map pass (SPEC-display-left-rail.md §2) - "AutofillCollapse header in
// rail: Superhero's stock .card-header 0.5rem 1rem (8/16) -> rail-scoped padding:7px 10px",
// now travelling with THIS call site (component-scoped) instead of RailRoot's own
// now-removed `.card-header` descendant-selector override - see AutofillCollapse.tsx's own
// `headerPadding` prop comment for the full rationale.
headerPadding="7px 10px"
>
{children}
</AutofillCollapse>
Expand Down Expand Up @@ -948,19 +954,19 @@ const ActionBarSearchGroup = styled.div`
// rail-scoped `padding:7px 10px`) - this was in the spec from the start but never actually
// landed as CSS (the comment this replaced claimed the demoted accordions deliberately "keep
// today's proven pattern," which was true for the CARD look/chrome but wrong for padding - the
// spec never carved padding out of scope). Scoped here (not a global AutofillCollapse.tsx
// change) via a plain `.card-header` descendant selector, which - at two class selectors deep
// under Emotion's own scoped RailRoot class - already outranks Bootstrap's bare `.card-header`
// rule without `!important`. Covers every rail-mounted AutofillCollapse: the five demoted
// RailSections below AND SourcesAccordion (§4 - "shell = AutofillCollapse"), both descendants of
// this same RailRoot node.
// spec never carved padding out of scope). Originally fixed here via a plain `.card-header`
// descendant selector (clobbering Bootstrap's bare global `.card-header` rule by selector
// specificity, not by scope - the exact "pinned in a higher location than expected" pattern
// SPEC-display-left-rail.md's "Source map addendum" flags as this fork's CSS-regression
// recurrence signature). CSS-fidelity source-map pass (follow-up): retired in favour of
// AutofillCollapse's own additive `headerPadding` prop, passed directly at each rail call site
// (RailSection/SourcesAccordion) - the value now travels WITH the component invocation instead
// of being injected from this ancestor wrapper two files away. See that prop's own comment in
// AutofillCollapse.tsx for the full before/after.
const RailRoot = styled.div`
.rail-head {
background: #22303f;
}
.card-header {
padding: 7px 10px;
}
.artist-line {
background: #22303f;
}
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/features/display/SourcesAccordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ export function SourcesAccordion() {
pad={2}
expanded={open}
onClick={() => setOpen((previous) => !previous)}
// CSS-fidelity source-map pass (SPEC-display-left-rail.md §2/§4) - see
// DisplayPage.tsx's RailSection's own identical `headerPadding` comment; this shell is
// the other AutofillCollapse mount in the rail ("shell = AutofillCollapse", §4).
headerPadding="7px 10px"
title={
<div className="d-flex flex-column flex-grow-1">
<div className="d-flex align-items-center gap-2">
Expand Down
171 changes: 171 additions & 0 deletions frontend/tests/DisplayLeftRailFidelity.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
import { expect } from "@playwright/test";

import {
cardDocumentsSelectVersionMixedResults,
castImplicitVoteSuccess,
defaultHandlers,
retractImplicitVoteSuccess,
searchResultsSelectVersionMixedResults,
sourceDocumentsOneResult,
submitTagVoteResolvesToApply,
tagConsensusTwoUnresolvedTags,
} from "@/mocks/handlers";

import { test } from "../playwright.setup";
import { openSelectVersionSection } from "./test-utils";

/**
* Permanent CSS-fidelity guard for the /display left rail
* (docs/proposals/mockups/proposal-h/SPEC-display-left-rail.md - SOURCE OF TRUTH for every
* literal value asserted below; that file's own §0 (theme tokens) and §2 (density table) are
* where these numbers come from, and its "Source map addendum" section documents where each one
* currently resolves from in the codebase (component-scoped inline style vs. a styled-component
* descendant selector vs. a genuinely global Bootstrap/theme default) - update BOTH files in the
* same change if a spec value ever changes, they're deliberately kept in lockstep, not just
* accurate at write time.
*
* WHY THIS EXISTS: PR #352 shipped several of §2's density-table rows as "done" in the spec's own
* prose while the actual CSS still fell through to Bootstrap's global defaults (the
* `AutofillCollapse` header's stock `0.5rem 1rem` instead of the rail's `7px 10px`; `gap-1`/
* `gap-2` instead of the mockup's literal `6px`; the Sources accordion's bulk-action row and list
* surface using unthemed Bootstrap defaults instead of the approved dark tokens) - then had to
* separately fix its own regression in a follow-up commit. That fix-round commit's own message is
* the exact failure mode this spec exists to make permanently visible: "several §2 density-table
* values never actually landed as CSS despite being documented as done." Every assertion below
* reads REAL computed styles (`toHaveCSS`, backed by `getComputedStyle`), never class names or
* inline-style source text - the same discipline `DisplaySlotStates.spec.ts` already follows for
* the sheet's own dark-state colors - so a future edit that silently reverts one of these values
* to a Bootstrap/global default fails this spec instead of shipping unnoticed.
*/

test.describe("Display left rail CSS fidelity guard (SPEC-display-left-rail.md)", () => {
test.describe.configure({ timeout: 60_000 });

const railFidelityHandlers = [
cardDocumentsSelectVersionMixedResults,
sourceDocumentsOneResult,
searchResultsSelectVersionMixedResults,
// The Attributes rail section fetches tag consensus the moment a slot is selected regardless
// of whether it's ever opened - see DisplayPage.spec.ts's own identical comment.
tagConsensusTwoUnresolvedTags,
submitTagVoteResolvesToApply,
castImplicitVoteSuccess,
retractImplicitVoteSuccess,
...defaultHandlers,
];

test("promoted zone, Select Version, and the unified filter/grid resolve the spec's literal §0/§2 values, not Bootstrap defaults", async ({
page,
network,
}) => {
network.use(...railFidelityHandlers);
await openSelectVersionSection(page);
await expect(page.getByTestId("display-rail-content")).toBeVisible();

// RailHeader `.rail-head` (§2: "p-2 (8px)" -> "padding:8px 10px, no bottom margin").
await expect(page.getByTestId("display-rail-header")).toHaveCSS(
"padding",
"8px 10px"
);

// `.artist-line` (§2: "px-2 py-1 (8/4)" -> "padding:8px 10px"; §0 promoted zone surface is
// $dark/$input-bg, #22303f = rgb(34, 48, 63)).
const artistLine = page.getByTestId("display-artist-section").locator("..");
await expect(artistLine).toHaveCSS("padding", "8px 10px");
await expect(artistLine).toHaveCSS("background-color", "rgb(34, 48, 63)");

// D14 confidence band `.d14` (§2: "margin:6px 0;padding:6px 8px;border-radius:6px chip" ->
// "margin:0;padding:8px 10px, full-width band, border-bottom" - kills the floating-chip
// inset margin; §3: confidence-chip surface #2b3e50 = rgb(43, 62, 80)).
const d14 = page.getByTestId("display-confidence-element");
await expect(d14).toBeVisible();
await expect(d14).toHaveCSS("margin", "0px");
await expect(d14).toHaveCSS("padding", "8px 10px");
await expect(d14).toHaveCSS("background-color", "rgb(43, 62, 80)");
await expect(d14).toHaveCSS("border-bottom", "1px solid rgb(22, 32, 43)");

// Select Version wrapper (§2: "px-2 pt-2 (8/8-top)" -> "padding:8px 10px").
const selectVersionWrapper = page
.locator(".select-version-heading")
.locator("..");
await expect(selectVersionWrapper).toHaveCSS("padding", "8px 10px");

// Unified Frame+Treatment filter fieldset (§6/§2: "padding:6px 8px; margin-bottom:6px"; §0
// raised surface #22303f = rgb(34, 48, 63), border rgba(0,0,0,.22)).
const fieldset = page.getByTestId("funnel-unified-filter");
await expect(fieldset).toBeVisible();
await expect(fieldset).toHaveCSS("padding", "6px 8px");
await expect(fieldset).toHaveCSS("margin-bottom", "6px");
await expect(fieldset).toHaveCSS("background-color", "rgb(34, 48, 63)");
await expect(fieldset).toHaveCSS("border", "1px solid rgba(0, 0, 0, 0.22)");

// The fieldset's own last `.ufilter .row` (Frame + Treatment sharing one row) and the
// continuous `.vgrid` result grid (§7/§2) both use the mockup's literal "gap:6px" - no exact
// Bootstrap spacing-scale match (`gap-1`=4px, `gap-2`=8px).
await expect(page.getByTestId("funnel-frame-treatment-row")).toHaveCSS(
"gap",
"6px"
);
await expect(page.getByTestId("select-version-continuous-grid")).toHaveCSS(
"gap",
"6px"
);

// Filters disclosure toggle (§8's "buttons-look-like-buttons" audit - a real button, not
// underlined text) - tightened to font-size:0.75rem/12px (owner fix round, "the buttons are
// too big").
const filtersToggle = page.getByTestId("funnel-filters-toggle");
await expect(filtersToggle).toHaveCSS("font-size", "12px");
expect(await filtersToggle.evaluate((el) => el.tagName)).toBe("BUTTON");
});

test("AutofillCollapse headers, Slot Actions, and the Sources accordion resolve the spec's literal §2/§4 values once expanded", async ({
page,
network,
}) => {
network.use(...railFidelityHandlers);
await openSelectVersionSection(page);
await expect(page.getByTestId("display-rail-content")).toBeVisible();

// AutofillCollapse header in the rail (§2: Superhero's stock `.card-header` "0.5rem 1rem"
// (8/16) -> rail-scoped "padding:7px 10px") - the header is always rendered regardless of
// collapse state, so no click is needed first. This is the exact regression PR #352's own
// fix-round commit describes: documented in the spec from the start, never actually landed
// as CSS the first time.
const cardDetailsHeader = page
.locator(".card-header")
.filter({ hasText: "Card Details" });
await expect(cardDetailsHeader).toHaveCSS("padding", "7px 10px");

// Slot Actions button stack (§2/§8: "button stack gap:6px", not `gap-2`'s 8px) - its content
// is gated behind the Collapse animation, so expand the section first.
await page
.locator(".card-header")
.filter({ hasText: "Slot Actions" })
.click();
const slotActions = page.getByTestId("display-slot-actions-section");
await expect(slotActions).toBeVisible();
await expect(slotActions).toHaveCSS("gap", "6px");

// Sources accordion (§4) - the bulk-action row and list surface: the mockup's own literal
// `.src-bulk{gap:6px;margin-bottom:6px}` and `.src-list{border:1px solid var(--border);
// background:var(--raised)}` (rgba(0,0,0,.22) / #22303f) - neither has an exact Bootstrap
// spacing/border-color match, and the raised surface is otherwise Bootstrap's stock `.border`
// gray, not the theme's own raised token.
await page
.getByTestId("display-sources-accordion")
.locator(".card-header")
.click();
const bulkRow = page
.getByTestId("display-sources-enable-all")
.locator("..");
await expect(bulkRow).toHaveCSS("gap", "6px");
await expect(bulkRow).toHaveCSS("margin-bottom", "6px");
const sourcesList = page.getByTestId("display-sources-list");
await expect(sourcesList).toHaveCSS(
"border",
"1px solid rgba(0, 0, 0, 0.22)"
);
await expect(sourcesList).toHaveCSS("background-color", "rgb(34, 48, 63)");
});
});
13 changes: 1 addition & 12 deletions frontend/tests/SelectVersionSection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
} from "@/mocks/handlers";

import { test } from "../playwright.setup";
import { importText, loadPageWithDefaultBackend } from "./test-utils";
import { openSelectVersionSection } from "./test-utils";

function buildRoute(route: string): string {
return `${localBackendURL}/${route}`;
Expand Down Expand Up @@ -52,17 +52,6 @@ const selectVersionHandlers = [
test.describe("SelectVersionSection (issue #167)", () => {
test.describe.configure({ timeout: 60_000 });

const openSelectVersionSection = async (
page: import("@playwright/test").Page
) => {
await loadPageWithDefaultBackend(page);
await importText(page, "my search query");
await page.getByRole("link", { name: "Editor" }).click();
await page.getByTestId("page-preview-slot").first().click();
// The rail always renders compressed tiles now (editor-completion package, E4/L9 - the
// toggle is gone entirely, hard-pinned true) - no "Compressed" click needed any more.
};

// Addendum item 2 (SPEC-display-left-rail.md §7, owner verbatim: "the 5 cards should be in 1
// section") - the old per-group wrapper divs (`select-version-group-*`,
// `select-version-printing-group-*`, `select-version-reason-group-*`) are GONE entirely. Group
Expand Down
15 changes: 15 additions & 0 deletions frontend/tests/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,21 @@ export const importText = async (page: Page, text: string) => {
).not.toBeVisible();
};

// Issue #167 (Select Version section) - navigates to the unified /display rail with a slot
// selected, so its always-open Select Version surface (and the rest of the promoted/demoted rail
// content around it) is on screen. Shared by SelectVersionSection.spec.ts (issue #167's own
// behavior coverage) and DisplayLeftRailFidelity.spec.ts (SPEC-display-left-rail.md's permanent
// CSS-fidelity guard - see that file's own module comment) so both drive the identical navigation
// path rather than each re-deriving it.
export const openSelectVersionSection = async (page: Page) => {
await loadPageWithDefaultBackend(page);
await importText(page, "my search query");
await page.getByRole("link", { name: "Editor" }).click();
await page.getByTestId("page-preview-slot").first().click();
// The rail always renders compressed tiles now (editor-completion package, E4/L9 - the
// toggle is gone entirely, hard-pinned true) - no "Compressed" click needed any more.
};

export async function expectCardSlotToExist(page: Page, slot: number) {
await expect(page.getByTestId(`front-slot${slot - 1}`)).toContainText(
`Slot ${slot}`
Expand Down
Loading