From b548611af13e43fc93c16f815f10ff37168ed985 Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Fri, 10 Jan 2025 12:00:15 +0100 Subject: [PATCH 1/5] chore: Adds feature metrics for collection prefs and button group --- .../__tests__/button-group-dev.test.tsx | 35 +++++++++++++++++++ src/button-group/index.tsx | 25 +++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/button-group/__tests__/button-group-dev.test.tsx b/src/button-group/__tests__/button-group-dev.test.tsx index c9d1d9d66f..503de19418 100644 --- a/src/button-group/__tests__/button-group-dev.test.tsx +++ b/src/button-group/__tests__/button-group-dev.test.tsx @@ -6,10 +6,13 @@ import { fireEvent } from '@testing-library/react'; import { warnOnce } from '@cloudscape-design/component-toolkit/internal'; import { ButtonGroupProps } from '../../../lib/components/button-group'; +import useBaseComponent from '../../../lib/components/internal/hooks/use-base-component'; import { renderButtonGroup } from './common'; import buttonStyles from '../../../lib/components/button/styles.css.js'; +jest.mock('../../../lib/components/internal/hooks/use-base-component', () => jest.fn().mockReturnValue({})); + jest.mock('@cloudscape-design/component-toolkit/internal', () => ({ ...jest.requireActual('@cloudscape-design/component-toolkit/internal'), warnOnce: jest.fn(), @@ -17,6 +20,7 @@ jest.mock('@cloudscape-design/component-toolkit/internal', () => ({ afterEach(() => { (warnOnce as jest.Mock).mockReset(); + (useBaseComponent as jest.Mock).mockReset(); }); const emptyGroup: ButtonGroupProps.ItemOrGroup[] = [ @@ -156,3 +160,34 @@ test('handles file upload', () => { expect(onFilesChange).toHaveBeenCalledWith(expect.objectContaining({ detail: { id: 'file', files: [file] } })); }); + +test('adds feature metrics', () => { + renderButtonGroup({ + items: [ + { type: 'icon-button', id: 'icon1', text: 'icon1' }, + { + type: 'group', + text: 'group1', + items: [ + { type: 'icon-toggle-button', id: 'toggle1', text: 'toggle1', pressed: false }, + { type: 'icon-toggle-button', id: 'toggle2', text: 'toggle2', pressed: false }, + ], + }, + { type: 'icon-file-input', id: 'file1', text: 'file1' }, + { type: 'menu-dropdown', id: 'menu1', text: 'menu1', items: [] }, + ], + }); + expect(useBaseComponent).toHaveBeenCalledWith('ButtonGroup', { + props: { + variant: 'icon', + dropdownExpandToViewport: undefined, + }, + metadata: { + iconButtonsCount: 1, + iconToggleButtonsCount: 2, + iconFileInputsCount: 1, + menuDropdownsCount: 1, + groupsCount: 1, + }, + }); +}); diff --git a/src/button-group/index.tsx b/src/button-group/index.tsx index c9da236c5a..5bfc2cac49 100644 --- a/src/button-group/index.tsx +++ b/src/button-group/index.tsx @@ -14,11 +14,19 @@ export { ButtonGroupProps }; const ButtonGroup = React.forwardRef( ({ variant, dropdownExpandToViewport, ...rest }: ButtonGroupProps, ref: React.Ref) => { const baseProps = getBaseProps(rest); + const itemCounts = getItemCounts(rest.items); const baseComponentProps = useBaseComponent('ButtonGroup', { props: { variant, dropdownExpandToViewport, }, + metadata: { + iconButtonsCount: itemCounts['icon-button'], + iconToggleButtonsCount: itemCounts['icon-toggle-button'], + iconFileInputsCount: itemCounts['icon-file-input'], + menuDropdownsCount: itemCounts['menu-dropdown'], + groupsCount: itemCounts.group, + }, }); const externalProps = getExternalProps(rest); @@ -35,5 +43,22 @@ const ButtonGroup = React.forwardRef( } ); +function getItemCounts(allItems: readonly ButtonGroupProps.ItemOrGroup[]) { + const counters = { 'icon-button': 0, 'icon-toggle-button': 0, 'icon-file-input': 0, 'menu-dropdown': 0, group: 0 }; + + function count(items: readonly ButtonGroupProps.ItemOrGroup[]) { + for (const item of items) { + counters[item.type] += 1; + + if (item.type === 'group') { + count(item.items); + } + } + } + count(allItems); + + return counters; +} + applyDisplayName(ButtonGroup, 'ButtonGroup'); export default ButtonGroup; From 7b3c17c416c7d77679f5d9b06bc4b6e316993019 Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Fri, 10 Jan 2025 12:10:43 +0100 Subject: [PATCH 2/5] smallfix for internal ssr tests that do not pass items although required --- src/button-group/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/button-group/index.tsx b/src/button-group/index.tsx index 5bfc2cac49..b2d7c88f9e 100644 --- a/src/button-group/index.tsx +++ b/src/button-group/index.tsx @@ -43,7 +43,7 @@ const ButtonGroup = React.forwardRef( } ); -function getItemCounts(allItems: readonly ButtonGroupProps.ItemOrGroup[]) { +function getItemCounts(allItems: readonly ButtonGroupProps.ItemOrGroup[] = []) { const counters = { 'icon-button': 0, 'icon-toggle-button': 0, 'icon-file-input': 0, 'menu-dropdown': 0, group: 0 }; function count(items: readonly ButtonGroupProps.ItemOrGroup[]) { From eae6af72ee69d19d10274766efdc20f892fcb938 Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Tue, 14 Jan 2025 18:33:21 +0100 Subject: [PATCH 3/5] provide default for expand to viewport prop --- .../snapshot-tests/__snapshots__/documenter.test.ts.snap | 1 + src/button-group/__tests__/button-group-dev.test.tsx | 2 +- src/button-group/index.tsx | 2 +- src/button-group/interfaces.ts | 5 ++++- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap b/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap index d0d91ee4b6..a3d0bb3fbf 100644 --- a/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap +++ b/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap @@ -3991,6 +3991,7 @@ Use this to provide a unique accessible name for each button group on the page." "type": "string", }, { + "defaultValue": "false", "description": "Use this property to determine dropdown placement strategy for all menu dropdown items. By default, the dropdown height is constrained to fit inside the height of its next scrollable container element. Enabling this property will allow the dropdown to extend beyond that container by using fixed positioning and diff --git a/src/button-group/__tests__/button-group-dev.test.tsx b/src/button-group/__tests__/button-group-dev.test.tsx index 503de19418..7b5edbdf20 100644 --- a/src/button-group/__tests__/button-group-dev.test.tsx +++ b/src/button-group/__tests__/button-group-dev.test.tsx @@ -180,7 +180,7 @@ test('adds feature metrics', () => { expect(useBaseComponent).toHaveBeenCalledWith('ButtonGroup', { props: { variant: 'icon', - dropdownExpandToViewport: undefined, + dropdownExpandToViewport: false, }, metadata: { iconButtonsCount: 1, diff --git a/src/button-group/index.tsx b/src/button-group/index.tsx index b2d7c88f9e..7c7ead96bd 100644 --- a/src/button-group/index.tsx +++ b/src/button-group/index.tsx @@ -12,7 +12,7 @@ import InternalButtonGroup from './internal'; export { ButtonGroupProps }; const ButtonGroup = React.forwardRef( - ({ variant, dropdownExpandToViewport, ...rest }: ButtonGroupProps, ref: React.Ref) => { + ({ variant, dropdownExpandToViewport = false, ...rest }: ButtonGroupProps, ref: React.Ref) => { const baseProps = getBaseProps(rest); const itemCounts = getItemCounts(rest.items); const baseComponentProps = useBaseComponent('ButtonGroup', { diff --git a/src/button-group/interfaces.ts b/src/button-group/interfaces.ts index 9b17a13eaf..9880828926 100644 --- a/src/button-group/interfaces.ts +++ b/src/button-group/interfaces.ts @@ -6,6 +6,7 @@ import { IconProps } from '../icon/interfaces'; import { BaseComponentProps } from '../internal/base-component'; import { NonCancelableEventHandler } from '../internal/events'; import { InternalBaseComponentProps } from '../internal/hooks/use-base-component'; +import { SomeRequired } from '../internal/types'; export interface ButtonGroupProps extends BaseComponentProps { /** @@ -97,7 +98,9 @@ export interface ButtonGroupProps extends BaseComponentProps { onFilesChange?: NonCancelableEventHandler; } -export interface InternalButtonGroupProps extends ButtonGroupProps, InternalBaseComponentProps {} +export interface InternalButtonGroupProps + extends SomeRequired, + InternalBaseComponentProps {} export namespace ButtonGroupProps { export type Variant = 'icon'; From 301e139f4de36f00f9d3d324641348d2408396cf Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Wed, 15 Jan 2025 17:01:42 +0100 Subject: [PATCH 4/5] jest spy on hook --- src/button-group/__tests__/button-group-dev.test.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/button-group/__tests__/button-group-dev.test.tsx b/src/button-group/__tests__/button-group-dev.test.tsx index 7b5edbdf20..61afa81325 100644 --- a/src/button-group/__tests__/button-group-dev.test.tsx +++ b/src/button-group/__tests__/button-group-dev.test.tsx @@ -6,11 +6,13 @@ import { fireEvent } from '@testing-library/react'; import { warnOnce } from '@cloudscape-design/component-toolkit/internal'; import { ButtonGroupProps } from '../../../lib/components/button-group'; -import useBaseComponent from '../../../lib/components/internal/hooks/use-base-component'; +import * as baseComponentHooks from '../../../lib/components/internal/hooks/use-base-component'; import { renderButtonGroup } from './common'; import buttonStyles from '../../../lib/components/button/styles.css.js'; +const useBaseComponentSpy = jest.spyOn(baseComponentHooks, 'default'); + jest.mock('../../../lib/components/internal/hooks/use-base-component', () => jest.fn().mockReturnValue({})); jest.mock('@cloudscape-design/component-toolkit/internal', () => ({ @@ -20,7 +22,7 @@ jest.mock('@cloudscape-design/component-toolkit/internal', () => ({ afterEach(() => { (warnOnce as jest.Mock).mockReset(); - (useBaseComponent as jest.Mock).mockReset(); + useBaseComponentSpy.mockReset(); }); const emptyGroup: ButtonGroupProps.ItemOrGroup[] = [ @@ -177,7 +179,7 @@ test('adds feature metrics', () => { { type: 'menu-dropdown', id: 'menu1', text: 'menu1', items: [] }, ], }); - expect(useBaseComponent).toHaveBeenCalledWith('ButtonGroup', { + expect(useBaseComponentSpy).toHaveBeenCalledWith('ButtonGroup', { props: { variant: 'icon', dropdownExpandToViewport: false, From 9ba450536aed1308b5c03fe67ffb77437456cde9 Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Thu, 16 Jan 2025 11:47:45 +0100 Subject: [PATCH 5/5] remove old mock --- src/button-group/__tests__/button-group-dev.test.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/button-group/__tests__/button-group-dev.test.tsx b/src/button-group/__tests__/button-group-dev.test.tsx index 61afa81325..5561eeb023 100644 --- a/src/button-group/__tests__/button-group-dev.test.tsx +++ b/src/button-group/__tests__/button-group-dev.test.tsx @@ -13,8 +13,6 @@ import buttonStyles from '../../../lib/components/button/styles.css.js'; const useBaseComponentSpy = jest.spyOn(baseComponentHooks, 'default'); -jest.mock('../../../lib/components/internal/hooks/use-base-component', () => jest.fn().mockReturnValue({})); - jest.mock('@cloudscape-design/component-toolkit/internal', () => ({ ...jest.requireActual('@cloudscape-design/component-toolkit/internal'), warnOnce: jest.fn(), @@ -22,7 +20,6 @@ jest.mock('@cloudscape-design/component-toolkit/internal', () => ({ afterEach(() => { (warnOnce as jest.Mock).mockReset(); - useBaseComponentSpy.mockReset(); }); const emptyGroup: ButtonGroupProps.ItemOrGroup[] = [