Skip to content

Add EuiComboBoxObject Playwright helper to @elastic/eui-test-helpers#9644

Open
steliosmavro wants to merge 6 commits into
elastic:mainfrom
steliosmavro:feat/test-helpers-combobox-component
Open

Add EuiComboBoxObject Playwright helper to @elastic/eui-test-helpers#9644
steliosmavro wants to merge 6 commits into
elastic:mainfrom
steliosmavro:feat/test-helpers-combobox-component

Conversation

@steliosmavro
Copy link
Copy Markdown

@steliosmavro steliosmavro commented May 8, 2026

Summary

First Playwright Component Object in @elastic/eui-test-helpers: EuiComboBoxObject. Implements Milestone 2 of the eui-test-helpers epic.

The plan is to wire one component (EuiComboBox) end-to-end first; subsequent helpers follow incrementally.

What's in this PR

  • BaseObject — base class for Playwright Component Objects. Stores scope, root, and testSubj.
  • EuiComboBoxObject — three minimal methods: selectOption(label), clear(), getSelectedOptions(). All idempotent; descriptive errors via expect.poll.
  • playwright.config.ts — defaults track kbn-scout's config (testIdAttribute: 'data-test-subj', conservative timeouts, retries: 0).
  • 5 validation tests including a multi-combo isolation test (new MultipleInstances story added to combo_box.stories.tsx).

Selector fix in selectors.ts

The existing OPTION / SELECTED_OPTION constants used [data-test-subj="comboBoxOptionsList"] (exact match). EUI propagates the consumer's data-test-subj to the options list as ${testSubj}-optionsList, joined via classNames — so the actual attribute value is "comboBoxOptionsList foo-optionsList" (whitespace-separated). Exact = doesn't match; ~= does.

Switched both to factory functions optionFor(testSubj, label?) / selectedOptionFor(testSubj, label?) using ~=. Verified by temp-breaking the scoping, observing the multi-combo test fail, and reverting.

Screenshots

N/A

Impact Assessment

  • 🔴 Breaking changes — Package is private: true, no external consumers.
  • 💅 Visual changes
  • 🧪 Test impact — Adds new tests; doesn't change existing behavior.
  • 🔧 Hard to integrate

Impact level: 🟢 None

Release Readiness

  • Documentation: packages/test-helpers/README.md updated with usage + contribution guide.
  • Figma:
  • Migration guide:
  • Adoption plan: M3/M4 in the same epic.

QA instructions for reviewer

# From repo root:
nvm use && yarn
cd packages/eui && yarn build:workspaces && yarn start  # Storybook on :6006

# In another terminal, from repo root:
yarn workspace @elastic/eui-test-helpers exec playwright install chromium
yarn workspace @elastic/eui-test-helpers test

Expected: 5/5 tests pass in ~3–4s.

Checklist before marking Ready for Review

  • Filled out all sections above
  • QA: light/dark modes, high contrast, mobile, browsers, keyboard, screen reader
  • QA: Tested in CodeSandbox and Kibana — Kibana consumption is M4.
  • QA: Tested docs changes
  • Tests: Validation tests added covering all public API + multi-combo isolation.
  • Changelog — Package is private, not released.
  • Breaking changes label

Reviewer checklist

  • Approved Impact Assessment
  • Approved Release Readiness

@steliosmavro
[@elastic/eui-test-helpers] Add EuiComboBox Playwright Component Object
061ff0b 
Implements milestone 2 of elastic#798. Adds BaseObject + EuiComboBoxObject with
selectOption / clear / getSelectedOptions, validation tests against EUI
Storybook (including a multi-combo isolation test using a new
MultipleInstances story), Playwright config aligned with kbn-scout, and
typescript lint via tsc --noEmit. Updates the existing selectors.ts to
fix `data-test-subj` matching: EUI propagates the consumer's
data-test-subj into the options list as a whitespace-joined token, so
exact `=` was broken; switched OPTION/SELECTED_OPTION to factory
functions using `~=`.
pheyos
pheyos reviewed May 8, 2026
View reviewed changes
Comment thread packages/test-helpers/src/components/combo_box/object.ts Outdated
@steliosmavro
[@elastic/eui-test-helpers] Rename selectOption to setSelectedOptions
c948849 
Per review feedback, switch the EuiComboBoxObject API from `selectOption(label)`
(add-semantic) to `setSelectedOptions(labels)` (replace-semantic). The setter
form is symmetric with `getSelectedOptions`, idempotent against the desired
end-state, and removes the ambiguity flagged in review around what
`selectOption` does when the label is already selected.

Implementation: naive replace (clear + add each), with set-equality short
circuit when current matches target. Per-label add logic extracted to a
private addOption() for readability. Adds a test that explicitly verifies
the replace semantic against an existing multi-selection.
steliosmavro
steliosmavro commented May 8, 2026
View reviewed changes
Comment thread packages/test-helpers/src/components/combo_box/object.ts
@steliosmavro
[@elastic/eui-test-helpers] Unify equality check in setSelectedOptions
5d99211 
Per review feedback, use sorted-array equality for both the early-return
short-circuit and the post-action guard. Previously the short-circuit
used length+every-includes (set-equality) while the guard used sort +
toEqual; same concept, two idioms. The unified form also lets us reuse
the sorted target array between the two checks.
@steliosmavro steliosmavro requested a review from pheyos May 12, 2026 02:57
dmlemeshko
dmlemeshko reviewed May 12, 2026
View reviewed changes
Comment thread packages/test-helpers/src/components/combo_box/object.spec.ts Outdated
Comment on lines +72 to +74
test('two combo boxes on the same page are operated independently', async ({
page,
}) => {
Copy link
Copy Markdown

@dmlemeshko dmlemeshko May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test might be better to place in its own spec? beforeEach doesn't really match here. If we plan to run tests in parallel, it might be good to have separate specs

Copy link
Copy Markdown
Author

@steliosmavro steliosmavro May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right 💯! Moved the multi-instance test to its own spec.

dmlemeshko
dmlemeshko reviewed May 12, 2026
View reviewed changes
Comment thread packages/test-helpers/src/components/combo_box/object.ts Outdated
Comment on lines +57 to +59
if (targetLabels.length > 0) {
// Close the dropdown so subsequent interactions start clean.
await this.root.page().keyboard.press('Escape');
Copy link
Copy Markdown

@dmlemeshko dmlemeshko May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does it work in practice? do we really need to press Escape everytime? what if the focus is in other UI element?

Copy link
Copy Markdown
Author

@steliosmavro steliosmavro May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! 🚀

Even scoping the 'Escape' event to the searchInput I don't think it would be enough as these kinds of events tend to bubble up so a modal on the consumer page could cattch it. Switched to await this.searchInput.blur().

@steliosmavro
Address Dima's review: split multi-instance spec, fix Escape side-effect
8906f82 
Replace page.keyboard.press('Escape') with searchInput.blur() — key events
bubble to document-level handlers (modal/flyout close listeners), blur does
not. Move the multi-instance isolation test to its own spec so beforeEach
is meaningful for every test in each file and specs run in parallel workers.
@steliosmavro
Merge remote-tracking branch 'upstream/main' into feat/test-helpers-c…
2493841 
…ombobox-component
@steliosmavro steliosmavro requested a review from dmlemeshko May 13, 2026 09:20
@steliosmavro steliosmavro marked this pull request as ready for review May 13, 2026 09:34
@steliosmavro steliosmavro requested a review from a team as a code owner May 13, 2026 09:34
@steliosmavro steliosmavro requested a review from tkajtoch May 13, 2026 09:35
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented May 13, 2026

💚 Build Succeeded

History

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented May 13, 2026

💚 Build Succeeded

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pheyos pheyos Awaiting requested review from pheyos

@dmlemeshko dmlemeshko Awaiting requested review from dmlemeshko

@tkajtoch tkajtoch Awaiting requested review from tkajtoch

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@steliosmavro @elasticmachine @pheyos @dmlemeshko