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
10 changes: 10 additions & 0 deletions .changeset/13091-declared-rules-reach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@object-ui/app-shell': patch
---

Setup's metadata lists keep a row's controls in reach on a narrow screen, and say in words when the activation state cannot be read (objectui#13091).

- **Pinned controls.** The list table scrolls inside the shared horizontal-overflow box. In the environment scope, the Active switch and, on the declared sharing rules, Clone are pinned to its right edge, with the seam shadow on the first of them while columns remain to the right. The Source column now comes before Active, so the pinned pair is the row's last two cells. At 1440 px the declared sharing rules no longer put Clone off-screen or cut the Active switch. The table draws its borders separated, with the row dividers on the cells, so no text scrolling beneath a pinned cell shows through its edge.
- **Unreadable state.** When the activation ledger read is refused or fails, the Active cell reads "Not visible to you" instead of `?`. Its tooltip carries "Activation state could not be read." followed by the server's own sentence for why, or the HTTP status when the server gave none.

One new string in the metadata-admin designer's own tables (en and zh): `engine.catalog.active.notVisible`. No export, prop or type member changes.
56 changes: 56 additions & 0 deletions e2e/live/declared-sharing-rules-reach-13091.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { test, expect, type Locator } from '@playwright/test';

/**
* objectui#13091 — the declared sharing rules list at 1440 px: the row's
* controls are in reach. Measured before the fix on a fresh showcase, signed in
* as admin: the first Clone button's box started at x=1515 (off-screen) and the
* table's scroll box cut the Active switch at its right edge.
*
* What is measured here, in a real browser against the running stack:
* - the first Clone button and the first Active switch both lie wholly inside
* the viewport AND inside the visible box of the table's scroller, with the
* table not scrolled at all;
* - when the table is wider than its box, the box says so
* (`data-overflow-right`, which drives the seam shadow on the pinned cells).
*
* The unit pins for the same page are in
* `ResourceListPage.declaredSharingRules-12982.test.tsx`.
*/
const LIST = '/apps/setup/metadata/sharing_rule?scope=environment';

async function boxOf(locator: Locator) {
const box = await locator.boundingBox();
expect(box, 'the element has a layout box').not.toBeNull();
return box!;
}

test.use({ viewport: { width: 1440, height: 1000 } });

test('at 1440 px, Clone and the Active switch are inside the viewport and the table box', async ({ page }) => {
await page.goto(LIST);
// The table's own scroll box, found by structure rather than by a test id,
// so the measurement reads the same element before and after the fix.
const scroller = page.locator('table').first().locator('xpath=..');
await expect(scroller).toBeVisible();
const clone = page.locator('[data-testid^="catalog-clone-"]').first();
const active = page.locator('[data-testid^="catalog-active-"]').first();
await expect(clone).toBeVisible();
await expect(active).toBeVisible();
expect(await scroller.evaluate((el) => el.scrollLeft)).toBe(0);

const viewport = page.viewportSize()!;
const frame = await boxOf(scroller);
for (const [what, control] of [['Clone', clone], ['Active', active]] as const) {
const b = await boxOf(control);
expect(b.x, `${what} starts inside the viewport`).toBeGreaterThanOrEqual(0);
expect(b.x + b.width, `${what} ends inside the viewport`).toBeLessThanOrEqual(viewport.width);
expect(b.x, `${what} starts inside the table box`).toBeGreaterThanOrEqual(frame.x);
expect(b.x + b.width, `${what} ends inside the table box`).toBeLessThanOrEqual(frame.x + frame.width + 0.5);
}

// The box reports its overflow, which is what draws the seam shadow.
const widths = await scroller.evaluate((el) => ({ scroll: el.scrollWidth, client: el.clientWidth }));
if (widths.scroll > widths.client + 1) {
await expect(scroller).toHaveAttribute('data-overflow-right', 'true');
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@
* declared rule off, which goes through the door; a rule switched off (no
* projection served) continues in the pre-filled New form;
* - the control: the permission-set catalog keeps its create affordance and
* shows no Clone column.
* shows no Clone column;
* - objectui#13091: the row's controls (the Active switch, Clone) are pinned
* to the right edge of the table's overflow box, which draws the seam
* shadow, so a 1440 or 1024 screen reaches them (the measured reach is
* `e2e/live/declared-sharing-rules-reach-13091.spec.ts`); and a principal
* the ledger read refuses reads a plain-language state, with the server's
* own sentence for why, never a bare "?".
*/
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
import { render, cleanup, screen, waitFor, fireEvent, within } from '@testing-library/react';
Expand Down Expand Up @@ -156,6 +162,8 @@ vi.mock('@object-ui/permissions', async (importOriginal) => {
});

let ledgerRows: Record<string, unknown>[] = [];
/** The ledger read's answer when it is refused; `null` answers `ledgerRows`. */
let ledgerRefusal: { status: number; body: unknown } | null = null;
let doorAnswer: { status: number; body: unknown } = { status: 200, body: {} };
/** `GET /sharing/rules/:name` per name; a missing name answers the recorded 404. */
let projections: Record<string, unknown> = {};
Expand All @@ -172,6 +180,7 @@ beforeEach(() => {
canAuthor = true;
canCreateRule = true;
ledgerRows = [];
ledgerRefusal = null;
doorAnswer = { status: 200, body: { success: true, data: { type: 'sharing_rule', enabled: false } } };
projections = { share_key_account_qualified_contacts_with_managers: KEY_ACCOUNT_PROJECTION };
createAnswer = (record) => ({ id: 'rec_1', ...record });
Expand All @@ -186,6 +195,7 @@ beforeEach(() => {
const json = (status: number, body: unknown) =>
new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } });
if (method === 'GET' && u.startsWith('/api/v1/data/sys_metadata_activation')) {
if (ledgerRefusal) return json(ledgerRefusal.status, ledgerRefusal.body);
return json(200, { object: 'sys_metadata_activation', records: ledgerRows, total: ledgerRows.length, hasMore: false });
}
if (method === 'POST' && u.startsWith('/api/v1/security/_activation/')) {
Expand Down Expand Up @@ -399,3 +409,85 @@ describe('Clone: a new-named organization rule pre-filled from the declaration',
expect(created).toEqual([]);
});
});

/** The data door's refusal of a ledger read to a principal without read on it. */
const LEDGER_READ_REFUSAL = "Permission denied: you cannot read 'sys_metadata_activation'.";

describe('a principal the ledger read refuses (objectui#13091)', () => {
it("reads a plain-language state per rule, with the server's sentence for why — never a bare '?'", async () => {
ledgerRefusal = { status: 403, body: { success: false, error: { code: 'PERMISSION_DENIED', message: LEDGER_READ_REFUSAL } } };
mount();
const cell = await screen.findByTestId(`catalog-active-unreadable-${KEY}`);
expect(cell).toHaveTextContent(t('engine.catalog.active.notVisible', ADMIN_LOCALE));
expect(cell.getAttribute('title')).toContain(LEDGER_READ_REFUSAL);
expect(cell.getAttribute('title')).toContain(t('engine.catalog.active.unknown', ADMIN_LOCALE));
expect(screen.getByTestId(`catalog-active-unreadable-${INQ}`)).toHaveTextContent(
t('engine.catalog.active.notVisible', ADMIN_LOCALE),
);
expect(screen.queryByText('?')).toBeNull();
// No switch is drawn on a state nobody read.
expect(screen.queryByTestId(`catalog-active-${KEY}`)).toBeNull();
});

it('a refusal without a sentence still reads in words, with the status for why', async () => {
ledgerRefusal = { status: 403, body: {} };
mount();
const cell = await screen.findByTestId(`catalog-active-unreadable-${KEY}`);
expect(cell).toHaveTextContent(t('engine.catalog.active.notVisible', ADMIN_LOCALE));
expect(cell.getAttribute('title')).toContain('HTTP 403');
});

it('control: a readable ledger draws the switch, and no unreadable state', async () => {
mount();
await screen.findByTestId(`catalog-active-${KEY}`);
expect(screen.queryByTestId(`catalog-active-unreadable-${KEY}`)).toBeNull();
});
});

describe("the row's controls stay in reach on a narrow screen (objectui#13091)", () => {
/** The cell (`td` / `th`) holding `el`. */
const cellOf = (el: Element) => el.closest('td, th') as HTMLElement;

it('Clone and the Active switch sit in cells pinned to the right edge of the overflow box', async () => {
mount();
const clone = cellOf(await screen.findByTestId(`catalog-clone-${KEY}`));
const active = cellOf(await screen.findByTestId(`catalog-active-${KEY}`));
const scroller = screen.getByTestId('metadata-list-scroller');
expect(scroller).toContainElement(clone);
// Clone holds the right edge at its fixed width; Active sits just left of it.
expect(clone).toHaveClass('sticky', 'right-0', 'w-28');
expect(active).toHaveClass('sticky', 'right-28');
// The pinned pair is the row's trailing pair, Active then Clone.
const row = clone.parentElement!;
expect(row.lastElementChild).toBe(clone);
expect(clone.previousElementSibling).toBe(active);
// Their header cells are pinned the same way, so the column heads stay with them.
const headers = Array.from(scroller.querySelectorAll('thead th'));
expect(headers[headers.length - 1]).toHaveClass('sticky', 'right-0');
expect(headers[headers.length - 2]).toHaveClass('sticky', 'right-28');
// Separated borders: in a collapsed-border table Chrome lets the text
// scrolling beneath a sticky cell show through its edge (measured live).
expect(scroller.querySelector('table')).toHaveClass('border-separate', 'border-spacing-0');
});

it('the first pinned cell draws the seam shadow, and the box leaves the shadow to it', async () => {
mount();
const active = cellOf(await screen.findByTestId(`catalog-active-${KEY}`));
const clone = cellOf(screen.getByTestId(`catalog-clone-${KEY}`));
expect(active.className).toContain('group-data-[overflow-right=true]/hscroll:before:opacity-100');
expect(clone.className).not.toContain('group-data-[overflow-right=true]/hscroll:before:opacity-100');
// `seam="pinned"`: no edge overlay competes with the seam.
expect(screen.getByTestId('metadata-list-scroller')).toHaveClass('group/hscroll');
expect(screen.queryByTestId('horizontal-overflow-shadow')).toBeNull();
});

it('without Clone, the Active column alone holds the right edge and draws the seam', async () => {
canCreateRule = false;
mount();
const active = cellOf(await screen.findByTestId(`catalog-active-${KEY}`));
expect(active).toHaveClass('sticky', 'right-0');
expect(active).not.toHaveClass('right-28');
expect(active.className).toContain('group-data-[overflow-right=true]/hscroll:before:opacity-100');
expect(active.parentElement!.lastElementChild).toBe(active);
});
});
Loading
Loading