Adds and exports test harnesses from SplitButton component #3055
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: f53b3c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds test harness utilities to the SplitButton component, providing a structured API for testing SplitButton functionality. The changes enable easier testing of SplitButton components by offering utility functions to find elements, interact with the menu, and check component states.
Key changes:
- Adds comprehensive test harness utilities with type definitions
- Updates the existing test suite to use the new test harnesses
- Exports the new testing utilities from the package index
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/split-button/tsconfig.json | Adds reference to test-harnesses tools |
| packages/split-button/src/testing/index.ts | Exports test utilities and types |
| packages/split-button/src/testing/getTestUtils.types.ts | Defines TypeScript interfaces for test utilities |
| packages/split-button/src/testing/getTestUtils.ts | Implements test utility functions for SplitButton |
| packages/split-button/src/testing/getTestUtils.spec.tsx | Tests for the test harness utilities |
| packages/split-button/src/index.ts | Exports test utilities from package |
| packages/split-button/src/SplitButton/SplitButton.spec.tsx | Refactored to use new test harnesses |
| packages/split-button/package.json | Adds test-harnesses dependency |
| return getAllByRole?.(menuEl, 'menuitem'); | ||
| }; | ||
|
|
||
| const queryMenuItems = (queryAllByRole?: Function) => { |
There was a problem hiding this comment.
The findAllByRole parameter should have proper typing instead of Function. Consider using the proper type from @testing-library/react: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => Promise<HTMLElement[]>
Suggested change
| const queryMenuItems = (queryAllByRole?: Function) => { | |
| const findMenuItems = async ( | |
| findAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => Promise<HTMLElement[]> | |
| ) => { | |
| const menuEl = await findMenu(); | |
| return findAllByRole?.(menuEl, 'menuitem'); | |
| }; | |
| const getMenuItems = ( | |
| getAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[] | |
| ) => { | |
| const menuEl = getMenu(); | |
| return getAllByRole?.(menuEl, 'menuitem'); | |
| }; | |
| const queryMenuItems = ( | |
| queryAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[] | |
| ) => { |
Copilot uses AI. Check for mistakes.
| return getAllByRole?.(menuEl, 'menuitem'); | ||
| }; | ||
|
|
||
| const queryMenuItems = (queryAllByRole?: Function) => { |
There was a problem hiding this comment.
The getAllByRole parameter should have proper typing instead of Function. Consider using the proper type from @testing-library/react: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[]
Suggested change
| const queryMenuItems = (queryAllByRole?: Function) => { | |
| const findMenuItems = async ( | |
| findAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[], | |
| ) => { | |
| const menuEl = await findMenu(); | |
| return findAllByRole?.(menuEl, 'menuitem'); | |
| }; | |
| const getMenuItems = ( | |
| getAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[], | |
| ) => { | |
| const menuEl = getMenu(); | |
| return getAllByRole?.(menuEl, 'menuitem'); | |
| }; | |
| const queryMenuItems = ( | |
| queryAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[], | |
| ) => { |
Copilot uses AI. Check for mistakes.
| return getAllByRole?.(menuEl, 'menuitem'); | ||
| }; | ||
|
|
||
| const queryMenuItems = (queryAllByRole?: Function) => { |
There was a problem hiding this comment.
The queryAllByRole parameter should have proper typing instead of Function. Consider using the proper type from @testing-library/react: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[]
Suggested change
| const queryMenuItems = (queryAllByRole?: Function) => { | |
| const findMenuItems = async ( | |
| findAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[], | |
| ) => { | |
| const menuEl = await findMenu(); | |
| return findAllByRole?.(menuEl, 'menuitem'); | |
| }; | |
| const getMenuItems = ( | |
| getAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[], | |
| ) => { | |
| const menuEl = getMenu(); | |
| return getAllByRole?.(menuEl, 'menuitem'); | |
| }; | |
| const queryMenuItems = ( | |
| queryAllByRole?: (container: HTMLElement, role: ByRoleMatcher, options?: ByRoleOptions) => HTMLElement[], | |
| ) => { |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Size Change: +3.5 kB (+0.18%) Total Size: 2 MB
ℹ️ View Unchanged
|
stephl3
reviewed
Aug 22, 2025
| @@ -33,12 +33,14 @@ | |||
| "@leafygreen-ui/palette": "workspace:^", | |||
| "@leafygreen-ui/polymorphic": "workspace:^", | |||
| "@leafygreen-ui/popover": "workspace:^", | |||
| "@leafygreen-ui/tokens": "workspace:^" | |||
| "@leafygreen-ui/tokens": "workspace:^", | |||
| "@lg-tools/test-harnesses": "workspace:^" | |||
There was a problem hiding this comment.
we need to add an exports key to this file so consumers can access the test utils
"exports": {
".": {
"require": "./dist/umd/index.js",
"import": "./dist/esm/index.js",
"types": "./dist/types/index.d.ts"
},
"./testing": {
"require": "./dist/umd/testing/index.js",
"import": "./dist/esm/testing/index.js",
"types": "./dist/types/testing/index.d.ts"
},
},
| }, | ||
| "peerDependencies": { | ||
| "@leafygreen-ui/leafygreen-provider": "workspace:^" | ||
| }, | ||
| "devDependencies": { | ||
| "@lg-tools/build": "workspace:^" | ||
|
|
| @@ -0,0 +1,27 @@ | |||
| import { FormUtils } from '@lg-tools/test-harnesses'; | |||
|
|
|||
| export interface GetTestUtilsReturnType<T extends HTMLElement = HTMLElement> { | |||
There was a problem hiding this comment.
This is a great start! I have a suggestion on the overall structure that would improve reusability and maintainability while following patterns we've established in other utils
Instead of a single flat API, we could structure this by composing utils from the button and menu packages. For reuse of Button test utils, you can refer to Code test utils on how this might be implemented. For Menu, this would be a great opportunity to create test utils there before writing the test utils for SplitButton
The utils for SplitButton might look something like this:
import { getTestUtils as getButtonTestUtils } from '@leafygreen-ui/button';
import { getTestUtils as getMenuTestUtils } from '@leafygreen-ui/menu';
const getTestUtils = (lgId) => {
const lgIds = getLgIds(lgId);
return {
getButtonUtils: () => getButtonTestUtils(lgIds.button),
getMenuUtils: () => getMenuTestUtils(lgIds.menu),
getTriggerUtils: () => getTriggerTestUtils(lgIds.trigger),
};
}
| /** | ||
| * Closes the menu by pressing Escape or clicking outside | ||
| */ | ||
| const closeMenu = async (options: { withKeyboard?: boolean } = {}) => { |
There was a problem hiding this comment.
should this also support closing by clicking the trigger?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✍️ Proposed changes
🎟 Jira ticket: Name of ticket
✅ Checklist
For new components
For bug fixes, new features & breaking changes
pnpm changesetand documented my changes🧪 How to test changes