Move Lite test IDs into Lite

[estib-vega]

Summary

• move LiteTestId from @gitbutler/ui into Lite UI source
• share the local test IDs with the instrumented React components and Lite E2E tests
• remove Lite's @gitbutler/ui dependency and the corresponding UI package step from the Lite E2E workflow

Why

Lite only depended on @gitbutler/ui for four test IDs. That caused the Lite E2E workflow to package unrelated Svelte UI code before testing the React app.

The local definition uses an erasable const object because Lite enables erasableSyntaxOnly.

[Copilot]

Pull request overview

This PR removes Lite’s dependency on @gitbutler/ui by relocating Lite-specific test IDs into the Lite React UI source, and updates Lite UI + Lite E2E tests to consume the local IDs. It also streamlines the Lite E2E CI job by dropping the unnecessary @gitbutler/ui packaging step.

Changes:

• Moved LiteTestId out of packages/ui and into apps/lite/ui/src/testIds.ts (as a const object).
• Updated Lite React routes and Lite E2E specs to import LiteTestId from the new Lite-local module.
• Removed @gitbutler/ui from Lite dependencies and removed the UI packaging step from the Lite E2E workflow job.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pnpm-lock.yaml	Removes Lite’s lockfile reference to @gitbutler/ui.
packages/ui/src/lib/utils/testIds.ts	Removes the Lite-only LiteTestId export from the shared UI package.
apps/lite/ui/src/testIds.ts	Introduces Lite-local LiteTestId constants for React components and tests to share.
apps/lite/ui/src/routes/project/$id/workspace/WorkspacePage.tsx	Switches LiteTestId import to the local Lite module.
apps/lite/ui/src/routes/project/$id/workspace/Outline.tsx	Switches LiteTestId import to the local Lite module.
apps/lite/ui/src/routes/index.tsx	Switches LiteTestId import to the local Lite module.
apps/lite/package.json	Drops @gitbutler/ui from Lite dependencies.
apps/lite/e2e/tests/start.spec.ts	Switches E2E import of LiteTestId to the Lite-local module.
apps/lite/e2e/tests/projects.spec.ts	Switches E2E import of LiteTestId to the Lite-local module.
apps/lite/e2e/tests/onboarding.spec.ts	Switches E2E import of LiteTestId to the Lite-local module.
.github/workflows/push.yaml	Removes pnpm --filter @gitbutler/ui package from the lite-e2e job build dependencies.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[samhh]

Perfect shouldn't be the enemy of good - any E2E test is an improvement - but worth noting that E2E test IDs can often be an anti-pattern. Better where possible to test against what the user actually sees, for example a button with the text "add project".

[estib-vega]

Better where possible to test against what the user actually sees, for example a button with the text "add project".

@samhh I disagree here. That would impose implicit constraints on the UI. We couldn't have buttons with the same label, and changing the labels would involve changing the tests.

I agree that there's a balance in between 0 and everything has a dedicated test ID, but I'm happy to look for that balance.
Is there a specific concern you have with Test IDs?

[samhh]

It does imply that changing copy also means updating tests.

Conflicts aren't often an issue since you can query within a subtree of the DOM, see e.g. Lite's branches spec. Arguably if copy conflicts it's signalling a UX issue.

It's better for maintaining accessibility, and slightly increases the coverage of what you're testing since you're more closely mimicking what the user sees and does. See:

• https://playwright.dev/docs/locators#:~:text=locators-,We%20recommend%20prioritizing%20role,technology%20perceive%20the%20page.,-Locate
• https://tkdodo.eu/blog/test-ids-are-an-a11y-smell
• https://testing-library.com/docs/queries/about/#priority

[estib-vega]

I get more your point now. Thanks for the links! :)

---

Some extra context

I think I've landed in the opinion of we shouldn't over do it, but there was a reason we had to:
In the previous GUI, we re arranged it a good 3 times (by hand without AI like cavemen) and that meant also having to find the right DOM selectors.

We had a very dense and high-depth component trees (are you selecting the stack container or the branch header?), with lots of repeated labels (e.g. the repeated 'start commit button').

We introduced the test IDs in order to make that process easier for us to change and to still have the confidence that we didn't get false positives

Conflicts aren't often an issue since you can query within a subtree of the DOM, see e.g. Lite's branches spec. Arguably if copy conflicts it's signaling a UX issue.

Probably. I think more than an issue, the nature of the application features we want to support demand a high UX price.
I think it's a bit better in lite, but since we're still trying out things, I expect that changing the UI/UX with the certainty that we can trust our E2E will be an important thing for us to have.

---

That said, I agree, adding test IDs for everything is not needed especially for buttons that probably won't be duplicate.
I'll remove some of the test IDs and merge this if that's ok.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (2)

apps/lite/e2e/tests/projects.spec.ts:3

• LiteTestId is imported but never used in this test file, which will typically fail TypeScript noUnusedLocals or ESLint no-unused-vars checks.

import path from "node:path";
import { LiteTestId } from "../../ui/src/testIds.ts";
import { expect, test } from "../test.ts";

apps/lite/ui/src/testIds.ts:3

• This file only defines OnboardingPage, but the PR description says Lite previously depended on @gitbutler/ui for four Lite test IDs and that they were moved into Lite. Either the missing IDs should be added here, or the PR description (and/or E2E strategy) should be updated to reflect that those IDs were intentionally removed in favor of role/name selectors.

export const LiteTestId = {
	OnboardingPage: "lite-onboarding-page",
} as const;

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (3)

apps/lite/ui/src/testIds.ts:3

• LiteTestId was previously a 4-value enum (in @gitbutler/ui) but the new local LiteTestId only defines OnboardingPage. That doesn’t match the PR description (“four test IDs”) and makes it harder to re-apply stable selectors in Lite UI/E2E if needed. Consider defining the full set here (even if some are not currently used) so Lite can consistently share these IDs across components and tests.

export const LiteTestId = {
	OnboardingPage: "lite-onboarding-page",
} as const;

apps/lite/ui/src/components/AddProjectButton.tsx:16

• AddProjectButton no longer accepts a testId prop, which prevents callers from attaching stable data-testid selectors. Given the PR goal of sharing Lite test IDs across UI + E2E, it’s safer to keep this optional prop so tests can use deterministic selectors without relying on user-visible button text.

type Props = {
	size?: ButtonSize;
	isPending: boolean;
	onClick: () => void;
};

export const AddProjectButton: FC<Props> = ({ size, isPending, onClick }) => (
	<button
		type="button"
		className={getButtonClassName({ size })}
		disabled={isPending}
		onClick={onClick}
	>

apps/lite/ui/src/routes/project/$id/workspace/SidebarHeader.tsx:98

• Removing the data-testid from the project picker trigger means Lite no longer exposes a stable selector for this control, despite the PR description aiming to share Lite test IDs between instrumented components and E2E tests. If you still want deterministic selectors, reintroduce data-testid (ideally using the new local LiteTestId) on this trigger.

				<Tooltip.Trigger
					aria-label={`${globalHotkeys.selectProject.meta.name} (current: ${p.project.title})`}
					className={classes(
						getButtonClassName({ variant: "ghost" }),
						"text-15",