[lokesh-bruno]

Ref BRU-4828

Description

Adds E2E coverage for the Collection Docs sidebar so the tree, collapse, drawer, and saved examples behave the same across navigation.

The coverage includes:

• The inline sidebar showing Overview, Environments, folders, and requests.
• Method badges on requests inside an expanded folder.
• The collapse icon staying hidden until the sidebar is hovered, and the main content widening when the sidebar collapses.
• Opening a folder to show its request, and closing it to hide that request.
• An empty folder expanding and collapsing without changing the rest of the tree.
• Folders, then requests, then scripts, in collection order.
• Deeply nested rows staying inside the sidebar, indented, with a long name truncated.
• Search and a direct link expanding the parent folders of a nested request.
• Expanded folders staying open after moving to Overview, and collapsing a parent leaving the open request on screen.
• The drawer opening from the hamburger, closing on navigation, and closing from the backdrop.
• Saved examples staying collapsed until their toggle is opened. Clicking one opens the request and highlights that example.
• A shared example URL and a reload restoring the highlight. Back and Forward move the highlight between examples.
• An unknown example slug falling back to the request page with no highlight.
• Opening a saved example from the drawer and keeping Next usable.

[changeset-bot]

🦋 Changeset detected

Latest commit: 4330d48

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@usebruno/api-docs	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[sundram-bruno]

the direct-link half of the test can never fail. The search step just before it has already opened Bookings and Lifecycle. page.goto to a URL that only changes after the # does not reload the page, so the open-folder state is still in memory, and the checks at lines 168 and 169 pass whatever the deep-link code does. Fix: close both folders first, or reload. Otherwise drop that half, because line 174 already covers a link opened in a fresh page.

[sundram-bruno]

This line used to check the navlink-method class, and now it checks the test id instead. The class is what styles the method column (width, font, alignment), so the spec no longer catches it if the class is dropped or renamed. Could we keep the class check and add the test-id check next to it, rather than replacing it?

Same at line 51 (the script JS badge). If the scoped ids from the other comment go in, the test-id line here becomes sidebar-item-method, with testId="sidebar-item" passed to the render.

[sundram-bruno]

These checks render SidebarNavLink without a testId and still expect sidebar-label / sidebar-method to be there. So the test locks in the fixed ids. If we move to ${testId}-label / ${testId}-method (see my comment on SidebarNavLink.tsx), could we update the spec to:

1. Pass a testId and check the scoped ids appear:

   const html = renderToStaticMarkup(<SidebarNavLink label="Cancel Booking" method="DELETE" testId="sidebar-item" />);
   expect(html).toContain('data-testid="sidebar-item-label"');
   expect(html).toContain('data-testid="sidebar-item-method"');

2. Add one case with no testId and check neither id renders:

   const html = renderToStaticMarkup(<SidebarNavLink label="Cancel Booking" method="DELETE" />);
   expect(html).not.toContain('-label"');
   expect(html).not.toContain('-method"');

[sundram-bruno]

The new test ids here are fixed strings, so every SidebarNavLink renders sidebar-label and sidebar-method, even when no testId is passed. Elsewhere in the app, child ids are built from the component's testId prop and left off when it is unset. See CopyButton.tsx:45 and Dropdown.tsx:59:

data-testid={testId ? `${testId}-button` : undefined}

Could we follow the same pattern here? Line 76:

Suggested change

	<span className="navlink-label" data-testid="sidebar-label">{label}</span>
	<span className="navlink-label" data-testid={testId ? `${testId}-label` : undefined}>{label}</span>

The same change applies to both method spans (lines 41 and 47):

data-testid={testId ? `${testId}-method` : undefined}

Every caller already passes a base id (sidebar-item, sidebar-example, sidebar-overview, sidebar-environments), so no row loses its ids. They just become scoped to the row type:

Row	Label id	Method id
request / folder	sidebar-item-label	sidebar-item-method
saved example	sidebar-example-label	n/a

Follow-ups this needs:

• e2e/components/sidebar.component.ts: update labels, item(), methodBadge() and exampleRow() to the scoped ids.
• e2e/tests/playground/method-selector.spec.ts: switch the four getByTestId('sidebar-method') calls to sidebar-item-method, ideally through a page-object method instead of chaining in the spec.
• SidebarNavLink.spec.tsx: pass a testId and assert ${testId}-label / ${testId}-method, and add a case with no testId that asserts neither id renders. Right now the spec asserts the fixed ids, which locks in the current behaviour.

[sundram-bruno]

page.getByRole('heading', { level: 1 }) can be replaced with requestPage.title, which the page object already has (request.page.ts:13, request-title).

[sundram-bruno]

page.getByRole('heading', { level: 1 }) can be replaced with requestPage.title, which the page object already has (request.page.ts:13, request-title).

[sundram-bruno]

the raw page.getByRole('main') for the content area should be a field on a page object.

[sundram-bruno]

This check only says "we are no longer on the example URL". Any navigation passes it, even a wrong one. I tried clicking Previous instead of Next here and the test still passed (it landed on #/billing/customers).

To prove Next works, could we check where it actually lands? From this example, Next goes to the "Get Customers - Filter by Email" request:

Suggested change

	await expect(page).not.toHaveURL(/200-ok-first-page/);
	await expect(page).toHaveURL(/#\/billing\/customers\/get-customers-filter-by-email$/);
	await expect(requestPage.title).toHaveText('Get Customers - Filter by Email');

With this, clicking Previous by mistake fails the test as expected. Line 117 (page.getByTestId('page')).toBeVisible()) can go too, since the title check already proves a page rendered.

[sundram-bruno]

This check can never fail at this screen size. AppShell.tsx only renders app-sidebar when the layout is desktop (isDesktop && !sidebarCollapsed), and desktop starts at 1024px. This test runs at 1023px, so the inline sidebar never exists here, before or after clicking Next.

It also isn't related to what the test is about (Next still working after opening an example from the drawer). "No inline sidebar below 1024px" is already covered in sidebar.spec.ts ("has no inline panel and opens the drawer from the hamburger"). Could we drop this line?

[sundram-bruno]

Could we put the prev/next links in a page object instead of using page.getByTestId('next-link') in the spec? Nothing wraps them yet (routing.spec.ts uses the raw ids too), so a small component would serve both.

e2e/components/layout/prev-next.component.ts:

import type { Page } from '@playwright/test';
import { BaseComponent } from '../base.component';

export class PrevNextComponent extends BaseComponent {
  constructor(page: Page) {
    super(page, page.getByTestId('prevnext'));
  }

  readonly previous = this.root.getByTestId('prev-link');
  readonly next = this.root.getByTestId('next-link');
}

Register it in e2e/playwright/pages.fixture.ts next to pageHeader:

prevNext: async ({ page }, use) => {
  await use(new PrevNextComponent(page));
},

Then add prevNext to this test's arguments and use:

Suggested change

	await page.getByTestId('next-link').click();
	await prevNext.next.click();

It's scoped to the prevnext wrapper, so it can't pick up a stray next-link elsewhere on the page.

[sundram-bruno]

SidebarComponent searches the whole page for sidebar-item rows. The playground renders the same SidebarTree (PlaygroundSidebar.tsx:74), with the same sidebar-item / sidebar-item-label test ids. Now that .first() is gone (good change), any sidebar lookup made while the playground is open matches two rows and fails Playwright's strict mode.

I checked it: open the playground on "get users", then sidebar.methodBadge('get users') fails with:

strict mode violation: ... resolved to 2 elements

No current test hits this, but the next person who uses sidebar in a playground test will.

Could we scope the component to the docs sidebar? Sidebar.tsx already renders data-testid="sidebar" on its root. Taking the root as a constructor argument also lets the playground reuse this component for its own tree (see my comment on playground.component.ts):

import type { Locator, Page } from '@playwright/test';

export class SidebarComponent extends BaseComponent {
  constructor(page: Page, root: Locator = page.getByTestId('sidebar')) {
    super(page, root);
  }

  readonly items = this.root.getByTestId('sidebar-item');
  ...
  readonly overview = this.root.getByTestId('sidebar-overview');
  readonly environments = this.root.getByTestId('sidebar-environments');

and in exampleRow:

    return this.root.getByTestId('sidebar-example').filter({

inline, collapseButton, expandButton, resizer, drawer, backdrop and hamburger live outside the sidebar root (in AppShell / the topbar), so they stay on this.page. With this change the same check resolves to 1 row, and the full e2e suite passes.

[sundram-bruno]

sidebarMethodBadge is built on sidebarItem, which matches any row whose name contains the text and takes the first one. So 'get users' would quietly pick a "get users by id" row if one is ever added above it. The docs version (SidebarComponent.methodBadge) matches the exact name.

Once SidebarComponent takes a root (see my comment on sidebar.component.ts), the playground can reuse it scoped to its own panel instead of keeping a second, looser copy:

import { SidebarComponent } from './sidebar.component';
...
  readonly sidebarPanel = this.page.getByTestId('playground-sidebar-panel');
  readonly sidebarTree = new SidebarComponent(this.page, this.sidebarPanel);

Suggested change

	sidebarMethodBadge(name: string): Locator {
	return this.sidebarItem(name).getByTestId('sidebar-item-method');
	}
	sidebarMethodBadge(name: string): Locator {
	return this.sidebarTree.methodBadge(name);

I ran method-selector.spec.ts with this and it passes. sidebarTree.item('get users') resolves to exactly one row, inside the playground panel.

[sundram-bruno]

main is part of the app layout (AppShell), not of any one page, but here it sits on BasePage. The sidebar collapse test then reaches it through overviewPage.main, which reads as if the test were about the overview page. It's also a role locator; the <main> has no test id yet (AppShell.tsx:182).

Could we give it a test id and expose it from a small layout component, like PrevNextComponent?

1. src/components/AppShell/AppShell.tsx, built from the existing testId prop (default app-shell):

   <main className="appshell-content" ref={contentRef} data-testid={`${testId}-content`}>

2. e2e/components/layout/app-shell.component.ts:

   import type { Page } from '@playwright/test';
   import { BaseComponent } from '../base.component';
   
   export class AppShellComponent extends BaseComponent {
     constructor(page: Page) {
       super(page, page.getByTestId('app-shell'));
     }
   
     readonly content = this.root.getByTestId('app-shell-content');
   }

3. Register appShell in pages.fixture.ts next to prevNext, and remove these two lines from BasePage.
4. In the collapse test (sidebar.spec.ts:49), take appShell instead of overviewPage and use appShell.content.boundingBox() for before and after.

The collapse test passes with this.

[sundram-bruno]

Two things on this check:

1. It's built in the spec. It chains .getByTestId('sidebar-item-label') onto sidebar.item(...) and hand-writes the scrollWidth > clientWidth check, which already exists as BaseComponent.isContentWiderThanBox (search.spec.ts:87 uses it).
2. It doesn't prove the name is truncated. scrollWidth > clientWidth only says the text is wider than its box. That stays true if text-overflow: ellipsis is removed from .navlink-label, which means the text gets cut off with no "...". I tried it: with the ellipsis removed, this test still passes.

Could we add an itemLabel helper in SidebarComponent, next to methodBadge:

  itemLabel(name: string): Locator {
    return this.item(name).getByTestId('sidebar-item-label');
  }

and change these lines to:

Suggested change

	const label = sidebar.item(longName).getByTestId('sidebar-item-label');
	const truncated = await label.evaluate((el) => el.scrollWidth > el.clientWidth);
	expect(truncated).toBe(true);
	const longLabel = sidebar.itemLabel(longName);
	expect(await sidebar.isContentWiderThanBox(longLabel)).toBe(true);
	await expect(longLabel).toHaveCSS('text-overflow', 'ellipsis');

With this, removing the ellipsis fails the test with Expected: "ellipsis", Received: "clip".

[sundram-bruno]

This order check can't catch scripts being mixed in with requests. In foldersCollection.ts, Setup Script has seq: 6 and Health Check has seq: 5. A bug that sorts scripts by seq together with requests puts the script after Health Check anyway, so the expected list is the same either way.

I tried it: I changed orderSiblings in navModel.ts to sort scripts by seq with the requests (...sortBySequence([...requests, ...files])), and this test still passed.

Could we give the script a lower seq than the request in src/e2eFixtures/foldersCollection.ts (line 127)?

    {
      name: 'Setup Script',
      type: 'script',
      seq: 4,

With the correct code the sidebar order doesn't change (scripts always go last), so the expected list here stays the same. With the bug, the script jumps above Health Check and the test fails. The full e2e suite passes with this fixture change, including routing.spec.ts, which relies on the prev/next order.

[sundram-bruno]

Small one, still here from last round: the script JS badge has a fixed inline style={{ color: 'var(--oc-colors-accent)' }}. The method span above only uses inline style because its color depends on the method at runtime. This one never changes, so it can be a class in StyledWrapper.ts. That also shortens this long line.

Suggested change

	<span className="navlink-leading navlink-method" data-testid={partTestId('method')} style={{ color: 'var(--oc-colors-accent)' }}>
	<span className="navlink-leading navlink-method navlink-script" data-testid={partTestId('method')}>

StyledWrapper.ts, after the .navlink-method block:

  .navlink-script {
    color: var(--oc-colors-accent);
  }

SidebarNavLink.spec.tsx, in the JS badge test:

    expect(html).toContain('navlink-script');
    expect(html).not.toMatch(/<span class="[^"]*navlink-script[^"]*"[^>]*style=/);

(The SSR output includes Emotion's <style> tag, so checking that the whole HTML doesn't contain the variable won't work. The pattern checks only the badge span's own style attribute.)

I compared the computed color before and after: rgb(185, 111, 29) in the default, hover and active states both ways, so nothing changes visually.