Skip to content

test(react-tags): add hook regression tests for Tag family#36229

Open
mainframev wants to merge 3 commits into
feat/headless-tag-1-refactorfrom
feat/headless-tag-2-regression-tests
Open

test(react-tags): add hook regression tests for Tag family#36229
mainframev wants to merge 3 commits into
feat/headless-tag-1-refactorfrom
feat/headless-tag-2-regression-tests

Conversation

@mainframev
Copy link
Copy Markdown
Contributor

@mainframev mainframev commented May 19, 2026

Stack

This PR is part of a 3-PR stack. Review and merge bottom-up:

  1. feat(react-tags): decouple useTagGroupBase_unstable from Tabster + export contexts #36228 — feat(react-tags): decouple useTagGroupBase_unstable from Tabster + export contexts (base: master) — merge first
  2. 👉 test(react-tags): add hook regression tests for Tag family #36229 — test(react-tags): add hook regression tests for Tag family (base: feat(react-tags): decouple useTagGroupBase_unstable from Tabster + export contexts #36228)depends on feat(react-tags): decouple useTagGroupBase_unstable from Tabster + export contexts #36228
  3. feat(react-headless-components-preview): add Tag family #36230 — feat(react-headless-components-preview): add Tag family (base: test(react-tags): add hook regression tests for Tag family #36229) — merge last; depends on test(react-tags): add hook regression tests for Tag family #36229

Summary

Adds renderHook-based regression tests for the Tag, InteractionTag, InteractionTagPrimary, InteractionTagSecondary, and TagGroup base + styled hooks. Locks in the public output shape so subsequent base-hook refactors (such as the Tabster decoupling in #36228) cannot silently regress the styled hooks.

Coverage highlights:

  • Slot element type via root.type (button vs span), event-handler presence/absence
  • Default DismissRegular injection happens only in the styled hooks (useTag_unstable, useInteractionTagSecondary_unstable), never in the base hooks
  • Context inheritance for appearance / size from TagGroupContext
  • Context override for disabled (group overrides tag prop)
  • New UseTagGroupBaseOptions contract: arrowNavigationProps is spread onto root, onAfterTagDismiss is invoked after a dismiss; both default to no-op
  • ARIA: aria-selected (listbox), aria-pressed (default), aria-labelledby (InteractionTagSecondary)

Stack created with GitHub Stacks CLIGive Feedback 💬

@github-actions
Copy link
Copy Markdown

Pull request demo site: URL

@mainframev mainframev force-pushed the feat/headless-tag-1-refactor branch from 99f8a77 to 24a9f85 Compare May 19, 2026 22:24
@mainframev mainframev force-pushed the feat/headless-tag-2-regression-tests branch from 36292c8 to f368d08 Compare May 19, 2026 22:24
@mainframev mainframev marked this pull request as ready for review May 20, 2026 01:11
@mainframev mainframev requested review from a team and ValentinaKozlova as code owners May 20, 2026 01:11
@mainframev mainframev force-pushed the feat/headless-tag-2-regression-tests branch from 77859e1 to 495dfb9 Compare May 20, 2026 01:36
@mainframev mainframev force-pushed the feat/headless-tag-2-regression-tests branch from 495dfb9 to 1d660cc Compare May 20, 2026 01:53
@mainframev mainframev force-pushed the feat/headless-tag-1-refactor branch from 24a9f85 to 4af8734 Compare May 20, 2026 02:04
@mainframev mainframev force-pushed the feat/headless-tag-2-regression-tests branch from 1d660cc to 14b9a10 Compare May 20, 2026 02:04
@tudorpopams tudorpopams requested a review from dmytrokirpa May 20, 2026 12:04
@mainframev mainframev force-pushed the feat/headless-tag-1-refactor branch from 4af8734 to 08cb5a5 Compare May 20, 2026 12:10
@mainframev mainframev force-pushed the feat/headless-tag-2-regression-tests branch from 14b9a10 to 28edd89 Compare May 20, 2026 12:10
Copy link
Copy Markdown
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

Not sure why tests are second in the stack. Also they could target master, no?

Comment on lines +54 to +61
it('should NOT expose design-only fields (appearance/shape/size) on base state', () => {
const ref = React.createRef<HTMLDivElement>();
const { result } = renderHook(() => useInteractionTagBase_unstable({}, ref), { wrapper: wrap() });

expect(result.current).not.toHaveProperty('appearance');
expect(result.current).not.toHaveProperty('shape');
expect(result.current).not.toHaveProperty('size');
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this test helpful?

import { useInteractionTag_unstable, useInteractionTagBase_unstable } from './useInteractionTag';

const wrap = (
contextOverrides: Parameters<typeof TagGroupContextProvider>[0]['value'] = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there is TagGroupContextValue

};

const wrap = (
overrides: Partial<Parameters<typeof InteractionTagContextProvider>[0]['value']> = {},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

InteractionTagContextValue?

Comment on lines +51 to +60
it('should NOT expose design-only fields (appearance/shape/size/avatar*) on base state', () => {
const ref = React.createRef<HTMLButtonElement>();
const { result } = renderHook(() => useInteractionTagPrimaryBase_unstable({}, ref), { wrapper: wrap() });

expect(result.current).not.toHaveProperty('appearance');
expect(result.current).not.toHaveProperty('shape');
expect(result.current).not.toHaveProperty('size');
expect(result.current).not.toHaveProperty('avatarShape');
expect(result.current).not.toHaveProperty('avatarSize');
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this helpful?

};

const wrap = (
overrides: Partial<Parameters<typeof InteractionTagContextProvider>[0]['value']> = {},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

InteractionTagContextValue?

it('should inject DismissRegular as default root children', () => {
const ref = React.createRef<HTMLButtonElement>();
const { result } = renderHook(() => useInteractionTagSecondary_unstable({}, ref), { wrapper: wrap() });
expect(result.current.root.children).toBeDefined();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's add at least React.isValidElement

expect(result.current.root.type).toBe('button');
});

it('should NOT inject DismissRegular children by default (icon injection lives in the styled hook)', () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
it('should NOT inject DismissRegular children by default (icon injection lives in the styled hook)', () => {
it('should not inject children by default', () => {

Comment on lines +86 to +92
it('should NOT expose design-only fields (appearance/shape/size)', () => {
const ref = React.createRef<HTMLButtonElement>();
const { result } = renderHook(() => useInteractionTagSecondaryBase_unstable({}, ref), { wrapper: wrap() });
expect(result.current).not.toHaveProperty('appearance');
expect(result.current).not.toHaveProperty('shape');
expect(result.current).not.toHaveProperty('size');
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this useful?

const { result } = renderHook(() => useTag_unstable({ dismissible: true }, ref), { wrapper: wrap() });

expect(result.current.dismissIcon).toBeDefined();
expect(result.current.dismissIcon?.children).toBeDefined();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's add React.isValidElement

expect(root.type).toBe('button');
});

it('should NOT inject a default dismissIcon children (icon injection lives in the styled hook)', () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto

const ref = React.createRef<HTMLDivElement>();
const { result } = renderHook(() => useTagGroup_unstable({}, ref));

expect((result.current.root as RootRecord)['data-tabster']).toEqual(expect.any(String));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

toHaveProperty should avoid type cast?

const arrowNavigationProps: TabsterDOMAttribute = { 'data-tabster': '{"mock":"value"}' };
const { result } = renderHook(() => useTagGroupBase_unstable({}, ref, { arrowNavigationProps }));

expect((result.current.root as RootRecord)['data-tabster']).toBe('{"mock":"value"}');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto

Comment on lines +61 to +66
it('should NOT throw when onDismiss/onAfterTagDismiss are omitted (true headless mode)', () => {
const ref = React.createRef<HTMLDivElement>();
const { result } = renderHook(() => useTagGroupBase_unstable({}, ref));

expect(() => result.current.handleTagDismiss({} as React.MouseEvent, { value: 'v1' })).not.toThrow();
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it useful?

Comment on lines +74 to +79
it('should NOT expose design-only fields (size, appearance) on base state', () => {
const ref = React.createRef<HTMLDivElement>();
const { result } = renderHook(() => useTagGroupBase_unstable({}, ref));
expect(result.current).not.toHaveProperty('size');
expect(result.current).not.toHaveProperty('appearance');
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it useful?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants