From 32dce4b7e8754d975eba64f53a560f3202a3340c Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Wed, 13 Mar 2024 18:45:01 -0500 Subject: [PATCH] Feedback --- .../tests/anchor-button-matrix.stories.ts | 24 +++--- .../src/anchor/tests/anchor-matrix.stories.ts | 28 +++++-- .../src/button/tests/button-matrix.stories.ts | 24 +++--- .../tests/menu-button-matrix.stories.ts | 59 ++++++++++---- .../tests/number-field-matrix.stories.ts | 26 ++++--- .../tests/toggle-button-matrix.stories.ts | 35 ++++++--- .../src/utilities/tests/matrix.ts | 77 ++++++++++++++++--- .../src/utilities/tests/states.ts | 14 +++- 8 files changed, 214 insertions(+), 73 deletions(-) diff --git a/packages/nimble-components/src/anchor-button/tests/anchor-button-matrix.stories.ts b/packages/nimble-components/src/anchor-button/tests/anchor-button-matrix.stories.ts index ce27d9209e..156dee48ca 100644 --- a/packages/nimble-components/src/anchor-button/tests/anchor-button-matrix.stories.ts +++ b/packages/nimble-components/src/anchor-button/tests/anchor-button-matrix.stories.ts @@ -14,7 +14,9 @@ import { disabledStates, DisabledState, InteractionState, - interactionStates + interactionStates, + nonInteractionStates, + disabledInteractionsFilter } from '../../utilities/tests/states'; import { createStory } from '../../utilities/tests/storybook'; import { hiddenWrapper } from '../../utilities/tests/hidden'; @@ -76,7 +78,7 @@ const component = ( export const anchorButtonThemeMatrix: StoryFn = createMatrixThemeStory( createMatrix(component, [ - interactionStates.slice(0, 1), + nonInteractionStates, disabledStates, appearanceStates, appearanceVariantStates, @@ -85,13 +87,17 @@ export const anchorButtonThemeMatrix: StoryFn = createMatrixThemeStory( ); export const anchorButtonInteractionsThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [ - interactionStates.slice(1), - disabledStates, - appearanceStates, - appearanceVariantStates, - [[false, true, false]] - ]) + createMatrix( + component, + [ + interactionStates, + disabledStates, + appearanceStates, + appearanceVariantStates, + [[false, true, false]] + ], + disabledInteractionsFilter + ) ); export const hiddenAnchorButton: StoryFn = createStory( diff --git a/packages/nimble-components/src/anchor/tests/anchor-matrix.stories.ts b/packages/nimble-components/src/anchor/tests/anchor-matrix.stories.ts index c9830f8629..ab2ed7185b 100644 --- a/packages/nimble-components/src/anchor/tests/anchor-matrix.stories.ts +++ b/packages/nimble-components/src/anchor/tests/anchor-matrix.stories.ts @@ -14,7 +14,9 @@ import { bodyFont } from '../../theme-provider/design-tokens'; import { anchorTag } from '..'; import { interactionStates, - type InteractionState + nonInteractionStates, + type InteractionState, + disabledInteractionsFilter } from '../../utilities/tests/states'; const metadata: Meta = { @@ -61,7 +63,7 @@ const component = ( export const anchorThemeMatrix: StoryFn = createMatrixThemeStory( createMatrix(component, [ - interactionStates.slice(0, 1), + nonInteractionStates, disabledStates, underlineHiddenStates, appearanceStates @@ -69,12 +71,22 @@ export const anchorThemeMatrix: StoryFn = createMatrixThemeStory( ); export const anchorInteractionsThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [ - interactionStates.slice(1), - disabledStates, - underlineHiddenStates, - appearanceStates - ]) + createMatrix( + component, + [ + interactionStates, + disabledStates, + underlineHiddenStates, + appearanceStates + ], + // A custom DisabledState type is used in this file, so we can't use the shared disabledInteractionsFilter + (interactionState: InteractionState, disabledState: DisabledState) => { + return ( + disabledState[0] !== 'Disabled' + || interactionState[0] === 'Hovered' + ); + } + ) ); export const hiddenAnchor: StoryFn = createStory( diff --git a/packages/nimble-components/src/button/tests/button-matrix.stories.ts b/packages/nimble-components/src/button/tests/button-matrix.stories.ts index ed96668227..c7caa65f31 100644 --- a/packages/nimble-components/src/button/tests/button-matrix.stories.ts +++ b/packages/nimble-components/src/button/tests/button-matrix.stories.ts @@ -11,7 +11,9 @@ import { disabledStates, DisabledState, InteractionState, - interactionStates + interactionStates, + nonInteractionStates, + disabledInteractionsFilter } from '../../utilities/tests/states'; import { createStory } from '../../utilities/tests/storybook'; import { hiddenWrapper } from '../../utilities/tests/hidden'; @@ -73,7 +75,7 @@ const component = ( export const buttonThemeMatrix: StoryFn = createMatrixThemeStory( createMatrix(component, [ - interactionStates.slice(0, 1), + nonInteractionStates, disabledStates, appearanceStates, appearanceVariantStates, @@ -82,13 +84,17 @@ export const buttonThemeMatrix: StoryFn = createMatrixThemeStory( ); export const buttonInteractionsThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [ - interactionStates.slice(1), - disabledStates, - appearanceStates, - appearanceVariantStates, - [[false, true, false]] - ]) + createMatrix( + component, + [ + interactionStates, + disabledStates, + appearanceStates, + appearanceVariantStates, + [[false, true, false]] + ], + disabledInteractionsFilter + ) ); export const hiddenButton: StoryFn = createStory( diff --git a/packages/nimble-components/src/menu-button/tests/menu-button-matrix.stories.ts b/packages/nimble-components/src/menu-button/tests/menu-button-matrix.stories.ts index 45924b26eb..47604a1ff4 100644 --- a/packages/nimble-components/src/menu-button/tests/menu-button-matrix.stories.ts +++ b/packages/nimble-components/src/menu-button/tests/menu-button-matrix.stories.ts @@ -11,7 +11,9 @@ import { disabledStates, DisabledState, InteractionState, - interactionStates + interactionStates, + nonInteractionStates, + disabledInteractionsFilter } from '../../utilities/tests/states'; import { createStory } from '../../utilities/tests/storybook'; import { hiddenWrapper } from '../../utilities/tests/hidden'; @@ -71,23 +73,50 @@ const component = ( `; export const menuButtonThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [ - interactionStates.slice(0, 1), - partVisibilityStates, - openStates, - disabledStates, - appearanceStates - ]) + createMatrix( + component, + [ + nonInteractionStates, + partVisibilityStates, + openStates, + disabledStates, + appearanceStates + ], + // Disabled and open is not a valid state + ( + _interactionState: InteractionState, + _partVisibilityState: PartVisibilityState, + openState: OpenState, + disabledState: DisabledState + ) => { + return disabledState[0] !== 'Disabled' || openState[0] !== 'Open'; + } + ) ); export const menuButtonInteractionsThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [ - interactionStates.slice(1), - [[false, true, false]], - openStates, - disabledStates, - appearanceStates - ]) + createMatrix( + component, + [ + interactionStates, + [[false, true, false]], + openStates, + disabledStates, + appearanceStates + ], + // Only interaction relevant to disabled controls is hover + ( + interactionState: InteractionState, + _partVisibilityState: PartVisibilityState, + _openState: OpenState, + disabledState: DisabledState + ) => { + return disabledInteractionsFilter( + interactionState, + disabledState + ); + } + ) ); export const hiddenMenuButton: StoryFn = createStory( diff --git a/packages/nimble-components/src/number-field/tests/number-field-matrix.stories.ts b/packages/nimble-components/src/number-field/tests/number-field-matrix.stories.ts index 2e10ec1f41..6cb1f2cfa3 100644 --- a/packages/nimble-components/src/number-field/tests/number-field-matrix.stories.ts +++ b/packages/nimble-components/src/number-field/tests/number-field-matrix.stories.ts @@ -13,7 +13,9 @@ import { errorStates, ErrorState, InteractionState, - interactionStates + interactionStates, + nonInteractionStates, + disabledInteractionsFilter } from '../../utilities/tests/states'; import { hiddenWrapper } from '../../utilities/tests/hidden'; import { NumberFieldAppearance } from '../types'; @@ -71,7 +73,7 @@ const component = ( export const numberFieldThemeMatrix: StoryFn = createMatrixThemeStory( createMatrix(component, [ - interactionStates.slice(0, 1), + nonInteractionStates, disabledStates, hideStepStates, valueStates, @@ -81,14 +83,18 @@ export const numberFieldThemeMatrix: StoryFn = createMatrixThemeStory( ); export const numberFieldInteractionsThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [ - interactionStates.filter(x => x[0] && !x[0].includes('Active')), - disabledStates, - hideStepStates.slice(0, 1), - valueStates.slice(1, 2), - errorStates.slice(0, 2), - appearanceStates - ]) + createMatrix( + component, + [ + interactionStates.filter(x => !x[0].includes('Active')), // skip irrelevant active states + disabledStates, + hideStepStates.filter(x => x[0] !== 'Hide Step'), // always show inc/dec buttons + valueStates.filter(x => x[0] !== 'Placeholder'), // don't test placeholder text + errorStates.filter(x => x[0] !== 'Error No Message'), // don't test error state w/o error text + appearanceStates + ], + disabledInteractionsFilter + ) ); export const hiddenNumberField: StoryFn = createStory( diff --git a/packages/nimble-components/src/toggle-button/tests/toggle-button-matrix.stories.ts b/packages/nimble-components/src/toggle-button/tests/toggle-button-matrix.stories.ts index 6cc1c7c7f3..c41141cea3 100644 --- a/packages/nimble-components/src/toggle-button/tests/toggle-button-matrix.stories.ts +++ b/packages/nimble-components/src/toggle-button/tests/toggle-button-matrix.stories.ts @@ -11,7 +11,9 @@ import { disabledStates, DisabledState, InteractionState, - interactionStates + interactionStates, + nonInteractionStates, + disabledInteractionsFilter } from '../../utilities/tests/states'; import { createStory } from '../../utilities/tests/storybook'; import { hiddenWrapper } from '../../utilities/tests/hidden'; @@ -73,7 +75,7 @@ const component = ( export const toggleButtonThemeMatrix: StoryFn = createMatrixThemeStory( createMatrix(component, [ - interactionStates.slice(0, 1), + nonInteractionStates, partVisibilityStates, checkedStates, disabledStates, @@ -82,13 +84,28 @@ export const toggleButtonThemeMatrix: StoryFn = createMatrixThemeStory( ); export const toggleButtonInteractionsThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [ - interactionStates.filter(x => x[0] && !x[0].includes('Active')), - [[false, true, false]], - checkedStates, - disabledStates, - appearanceStates - ]) + createMatrix( + component, + [ + interactionStates, + [[false, true, false]], + checkedStates, + disabledStates, + appearanceStates + ], + // Only interaction relevant to disabled controls is hover + ( + interactionState: InteractionState, + _partVisibilityState: PartVisibilityState, + _checkedState: CheckedState, + disabledState: DisabledState + ) => { + return disabledInteractionsFilter( + interactionState, + disabledState + ); + } + ) ); export const hiddenButton: StoryFn = createStory( diff --git a/packages/nimble-components/src/utilities/tests/matrix.ts b/packages/nimble-components/src/utilities/tests/matrix.ts index c98dc83b0b..959b9ffd0e 100644 --- a/packages/nimble-components/src/utilities/tests/matrix.ts +++ b/packages/nimble-components/src/utilities/tests/matrix.ts @@ -30,12 +30,14 @@ export function createMatrix(component: () => ViewTemplate): ViewTemplate; export function createMatrix( component: (state1: State1) => ViewTemplate, - dimensions: readonly [readonly State1[]] + dimensions: readonly [readonly State1[]], + filter?: (state1: State1) => boolean ): ViewTemplate; export function createMatrix( component: (state1: State1, state2: State2) => ViewTemplate, - dimensions: readonly [readonly State1[], readonly State2[]] + dimensions: readonly [readonly State1[], readonly State2[]], + filter?: (state1: State1, state2: State2) => boolean ): ViewTemplate; export function createMatrix( @@ -44,7 +46,8 @@ export function createMatrix( readonly State1[], readonly State2[], readonly State3[] - ] + ], + filter?: (state1: State1, state2: State2, state3: State3) => boolean ): ViewTemplate; export function createMatrix( @@ -59,7 +62,13 @@ export function createMatrix( readonly State2[], readonly State3[], readonly State4[] - ] + ], + filter?: ( + state1: State1, + state2: State2, + state3: State3, + state4: State4 + ) => boolean ): ViewTemplate; export function createMatrix( @@ -76,7 +85,14 @@ export function createMatrix( readonly State3[], readonly State4[], readonly State5[] - ] + ], + filter?: ( + state1: State1, + state2: State2, + state3: State3, + state4: State4, + state5: State5 + ) => boolean ): ViewTemplate; export function createMatrix( @@ -95,7 +111,15 @@ export function createMatrix( readonly State4[], readonly State5[], readonly State6[] - ] + ], + filter?: ( + state1: State1, + state2: State2, + state3: State3, + state4: State4, + state5: State5, + state6: State6 + ) => boolean ): ViewTemplate; export function createMatrix< @@ -124,7 +148,16 @@ export function createMatrix< readonly State5[], readonly State6[], readonly State7[] - ] + ], + filter?: ( + state1: State1, + state2: State2, + state3: State3, + state4: State4, + state5: State5, + state6: State6, + state7: State7 + ) => boolean ): ViewTemplate; export function createMatrix< @@ -156,7 +189,17 @@ export function createMatrix< readonly State6[], readonly State7[], readonly State8[] - ] + ], + filter?: ( + state1: State1, + state2: State2, + state3: State3, + state4: State4, + state5: State5, + state6: State6, + state7: State7, + state8: State8 + ) => boolean ): ViewTemplate; export function createMatrix< @@ -191,12 +234,24 @@ export function createMatrix< readonly State7[], readonly State8[], readonly State9[] - ] + ], + filter?: ( + state1: State1, + state2: State2, + state3: State3, + state4: State4, + state5: State5, + state6: State6, + state7: State7, + state8: State8, + state9: State9 + ) => boolean ): ViewTemplate; export function createMatrix( component: (...states: readonly unknown[]) => ViewTemplate, - dimensions?: readonly (readonly unknown[])[] + dimensions?: readonly (readonly unknown[])[], + filter?: (...states: readonly unknown[]) => boolean ): ViewTemplate { const matrix: ViewTemplate[] = []; const recurseDimensions = ( @@ -208,7 +263,7 @@ export function createMatrix( for (const currentState of currentDimension!) { recurseDimensions(remainingDimensions, ...states, currentState); } - } else { + } else if (!filter || filter(...states)) { matrix.push(component(...states)); } }; diff --git a/packages/nimble-components/src/utilities/tests/states.ts b/packages/nimble-components/src/utilities/tests/states.ts index 9d8647cf79..72387d0f85 100644 --- a/packages/nimble-components/src/utilities/tests/states.ts +++ b/packages/nimble-components/src/utilities/tests/states.ts @@ -42,8 +42,8 @@ export type ReadOnlyState = (typeof readOnlyStates)[number]; export const iconVisibleStates = [false, true] as const; export type IconVisibleState = (typeof iconVisibleStates)[number]; +export const nonInteractionStates = [['', '']] as const; export const interactionStates = [ - ['', ''], // Using pseudo-*-all will turn on the effect for any nested custom elements (e.g. inc/dec buttons in number field). // It works around the limitation that selectors for shadow elements can only see the immediate host's classes. ['Hovered', 'hover pseudo-hover-all'], @@ -54,4 +54,14 @@ export const interactionStates = [ 'focus-visible focus-within pseudo-focus-visible-all pseudo-focus-within-all' ] ] as const; -export type InteractionState = (typeof interactionStates)[number]; +export type InteractionState = + | (typeof interactionStates)[number] + | (typeof nonInteractionStates)[number]; + +// Only interaction relevant to disabled controls is hover +export function disabledInteractionsFilter( + interactionState: InteractionState, + disabledState: DisabledState +): boolean { + return disabledState[0] !== 'Disabled' || interactionState[0] === 'Hovered'; +}