Skip to content

Commit

Permalink
chore!: remove deprecated useTrapFocus arrowKeysOn option [SFUI2-1273] (
Browse files Browse the repository at this point in the history 
#3008)

BREAKING CHANGE: remove deprecated useTrapFocus arrowKeysOn option
  • Loading branch information
@Szymon-dziewonski
Szymon-dziewonski authored Oct 6, 2023
1 parent bbec9e1 commit 727a192
Show file tree
Hide file tree
Showing 12 changed files with 146 additions and 76 deletions.
8 changes: 8 additions & 0 deletions .changeset/mighty-monkeys-occur.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@storefront-ui/react': major
'@storefront-ui/vue': major
---

BREAKING CHANGE: Deprecated option `arrowKeysOn` from `useTrapFocus` is removed.
This option was separated and replaced by two more specialised options `arrowKeysLeftRight` and `arrowKeysUpDown`.
In order to migrate please one of those options or both.
6 changes: 0 additions & 6 deletions apps/docs/components/hooks/useTrapFocus.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,6 @@ useTrapFocus(focusTrapElementRef)

:::::: slot api

<!-- TODO: remove arrowKeysOn before 3.0.0 release -->
::: warning DEPRECATION
Parameter `arrowKeysOn` will be deprecated since version 2.3 and removed in next major version.
:::

## Parameters

| Name | Type | Default value | Description |
Expand All @@ -98,7 +93,6 @@ Parameter `arrowKeysOn` will be deprecated since version 2.3 and removed in next
| activeState | `boolean` | `true` | Mount `useTrapFocus` when active is `true` |
<!-- end react -->
| initialFocus | `number | 'autofocus' | 'container'` | `0` | index number of desired focus element on init, `autofocus` for first marked element with attribute `autofocus`, `container` so `refElement` would be initially focused, `false` for disabling this option |
| arrowKeysOn | `boolean` | `false` | (deprecated) Enable/Disable possibility of using keyboard arrows `left`/`right` for jumping through focusable elements |
| arrowKeysLeftRight | `boolean` | `false` | Enable/Disable possibility of using keyboard arrows `left | up`/`right | down` for jumping through focusable elements |
| arrowKeysUpDown | `boolean` | `false` | Enable/Disable possibility of using keyboard arrows `up`/`down` for jumping through focusable elements |
| initialFocusContainerFallback | `boolean` | `false` | Fallback for initial focus |
Expand Down
3 changes: 2 additions & 1 deletion apps/preview/next/pages/showcases/SelectDropdown/SelectDropdownDisabled.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ export default function SelectDropdownDisabled() {
const { refs, style: dropdownStyle } = useDropdown({ isOpen, onClose: close });

useTrapFocus(refs.floating, {
arrowKeysOn: true,
arrowKeysLeftRight: true,
arrowKeysUpDown: true,
activeState: isOpen,
initialFocusContainerFallback: true,
});
Expand Down
3 changes: 2 additions & 1 deletion apps/preview/nuxt/pages/showcases/FormFields/FormFieldsRequired.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,8 @@ const {
});
useTrapFocus(floatingRef as Ref<HTMLUListElement>, {
arrowKeysOn: true,
arrowKeysLeftRight: true,
arrowKeysUpDown: true,
activeState: dropdownOpen,
initialFocusContainerFallback: true,
});
Expand Down
10 changes: 0 additions & 10 deletions packages/sfui/frameworks/react/hooks/useTrapFocus/useTrapFocus.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ type UseTrapFocusOptions = TabbableOptions &
activeState?: boolean;
initialFocus?: number | `${InitialFocusType}` | false;
initialFocusContainerFallback?: boolean;
/**
* Enabling both `left` | `up` | `right` | `down` arrow keys.
* @deprecated Since version 2.3. Use arrowKeysLeftRight or/and arrowKeysUpDown options instead.
*/
arrowKeysOn?: boolean;
arrowKeysLeftRight?: boolean;
arrowKeysUpDown?: boolean;
};
Expand All @@ -29,7 +24,6 @@ const defaultOptions = {
activeState: true,
initialFocus: 0,
initialFocusContainerFallback: false,
arrowKeysOn: false,
arrowKeysLeftRight: false,
arrowKeysUpDown: false,
};
Expand All @@ -41,7 +35,6 @@ export const useTrapFocus = (containerElementRef: RefObject<HTMLElement | null>,
includeContainer,
activeState,
initialFocus,
arrowKeysOn,
arrowKeysLeftRight,
arrowKeysUpDown,
initialFocusContainerFallback,
Expand Down Expand Up @@ -86,9 +79,6 @@ export const useTrapFocus = (containerElementRef: RefObject<HTMLElement | null>,
arrowFocusGroupSelector && containerElementRef.current?.querySelector(arrowFocusGroupSelector);
const additionalData = isAnyGroupElement ? { arrowFocusGroupSelector } : {};

if (arrowKeysOn && (event.key === 'ArrowLeft' || event.key === 'ArrowUp')) focusPreviousItem({ additionalData });
if (arrowKeysOn && (event.key === 'ArrowRight' || event.key === 'ArrowDown')) focusNextItem({ additionalData });

if (arrowKeysLeftRight && event.key === 'ArrowLeft') focusPreviousItem({ additionalData });
if (arrowKeysLeftRight && event.key === 'ArrowRight') focusNextItem({ additionalData });
if (arrowKeysUpDown && event.key === 'ArrowUp') focusPreviousItem({ additionalData });
Expand Down
10 changes: 0 additions & 10 deletions packages/sfui/frameworks/vue/composables/useTrapFocus/useTrapFocus.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ type UseTrapFocusOptions = TabbableOptions &
activeState?: Ref<boolean>;
initialFocus?: number | `${InitialFocusType}` | false;
initialFocusContainerFallback?: boolean;
/**
* Enabling both `left` | `up` | `right` | `down` arrow keys.
* @deprecated Since version 2.3. Use arrowKeysLeftRight or/and arrowKeysUpDown options instead.
*/
arrowKeysOn?: boolean;
arrowKeysLeftRight?: boolean;
arrowKeysUpDown?: boolean;
};
Expand All @@ -38,7 +33,6 @@ const defaultOptions = {
activeState: ref(true),
initialFocus: 0,
initialFocusContainerFallback: false,
arrowKeysOn: false,
arrowKeysLeftRight: false,
arrowKeysUpDown: false,
};
Expand All @@ -53,7 +47,6 @@ export const useTrapFocus = (
includeContainer,
activeState,
initialFocus,
arrowKeysOn,
arrowKeysLeftRight,
arrowKeysUpDown,
initialFocusContainerFallback,
Expand Down Expand Up @@ -101,9 +94,6 @@ export const useTrapFocus = (
const isAnyGroupElement = arrowFocusGroupSelector && containerHTMLElement?.querySelector(arrowFocusGroupSelector);
const additionalData = isAnyGroupElement ? { arrowFocusGroupSelector } : {};

if (arrowKeysOn && (event.key === 'ArrowLeft' || event.key === 'ArrowUp')) focusPreviousItem({ additionalData });
if (arrowKeysOn && (event.key === 'ArrowRight' || event.key === 'ArrowDown')) focusNextItem({ additionalData });

if (arrowKeysLeftRight && event.key === 'ArrowLeft') focusPreviousItem({ additionalData });
if (arrowKeysLeftRight && event.key === 'ArrowRight') focusNextItem({ additionalData });
if (arrowKeysUpDown && event.key === 'ArrowUp') focusPreviousItem({ additionalData });
Expand Down
7 changes: 6 additions & 1 deletion packages/tests/components/SfButton/SfButton.PageObject.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@ export default class SfButtonBaseObject extends BasePage {
return this;
}

isSquare(value) {
hasClass(value: string) {
this.container.should('have.class', value);
return this;
}

doesNotHaveType() {
this.container.should('not.have.attr', 'type');
return this;
}
}
127 changes: 85 additions & 42 deletions packages/tests/components/SfButton/SfButton.cy.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@ describe('SfButton', () => {
slotPrefix?: boolean;
slotSuffix?: boolean;
slotDefault?: boolean;
tag?: string;
};
const initializeComponent = ({
size = SfButtonSize.base,
variant = SfButtonVariant.primary,
size,
variant,
disabled,
square,
slotPrefix,
slotSuffix,
slotDefault = true,
tag,
}: InitializeComponentParams = {}) => {
return mount({
vue: {
Expand All @@ -46,6 +48,7 @@ describe('SfButton', () => {
variant,
disabled,
square,
tag,
},
slots: {
...(slotPrefix && { prefix: '<SfIconCheckCircleVue/>' }),
Expand All @@ -61,6 +64,7 @@ describe('SfButton', () => {
square={square}
slotPrefix={slotPrefix && <SfIconCheckCircleReact />}
slotSuffix={slotSuffix && <SfIconIndeterminateCheckBoxReact />}
as={tag}
>
{slotDefault ? slotDefaultContent : undefined}
</SfButtonReact>
Expand Down Expand Up @@ -90,6 +94,24 @@ describe('SfButton', () => {
});
});

const squarePaddingsToSize = {
[SfButtonSize.sm]: 'p-1.5',
[SfButtonSize.lg]: 'p-3',
[SfButtonSize.base]: 'p-2',
};

describe('when prop square=true and size is set to ', () => {
Object.values(SfButtonSize).forEach((componentSize) => {
describe(`${componentSize}`, () => {
it(`should render correct square ${componentSize} size`, () => {
initializeComponent({ size: componentSize, square: true });

page().hasClass(squarePaddingsToSize[componentSize]).makeSnapshot();
});
});
});
});

describe('when prop variant is set to ', () => {
Object.values(SfButtonVariant).forEach((componentVariant) => {
describe(`${componentVariant}`, () => {
Expand All @@ -100,69 +122,90 @@ describe('SfButton', () => {
});
});
});
});

describe('when prop disabled=true', () => {
it(`should render as disabled`, () => {
initializeComponent({ disabled: true });
describe('when no size specified via props', () => {
it(`should init with SfButtonSize.base size`, () => {
initializeComponent();

page().isDisabled().makeSnapshot();
});
page().hasClass('py-2').hasClass('leading-6').hasClass('px-4').hasClass('gap-2').makeSnapshot();
});
});

describe('when prop square=true', () => {
it(`should render as square button`, () => {
initializeComponent({ square: true, size: SfButtonSize.base });
describe('when no variant specified via props', () => {
it(`should init with SfButtonVariant.primary variant`, () => {
initializeComponent();

page().isSquare('p-2').makeSnapshot();
});
page()
.hasClass('bg-primary-700')
.hasClass('hover:bg-primary-800')
.hasClass('active:bg-primary-900')
.hasClass('disabled:bg-disabled-300')
.makeSnapshot();
});
});

describe('when only prefix', () => {
it(`should render square button`, () => {
initializeComponent({ slotPrefix: true, slotDefault: false });
describe('when prop disabled=true', () => {
it(`should render as disabled`, () => {
initializeComponent({ disabled: true });

page().makeSnapshot();
});
page().isDisabled().makeSnapshot();
});
});

describe('when only prefix', () => {
it(`should render square button`, () => {
initializeComponent({ slotPrefix: true, slotDefault: false });

page().makeSnapshot();
});
});

describe('when only suffix', () => {
it(`should render square button`, () => {
initializeComponent({ slotSuffix: true, slotDefault: false });
describe('when only suffix', () => {
it(`should render square button`, () => {
initializeComponent({ slotSuffix: true, slotDefault: false });

page().makeSnapshot();
});
page().makeSnapshot();
});
});

describe('when suffix and prefix', () => {
it(`should render button with same gaps`, () => {
initializeComponent({ slotPrefix: true, slotSuffix: true, slotDefault: false });
describe('when suffix and prefix', () => {
it(`should render button with same gaps`, () => {
initializeComponent({ slotPrefix: true, slotSuffix: true, slotDefault: false });

page().makeSnapshot();
});
page().makeSnapshot();
});
});

describe('when suffix, content and prefix', () => {
it(`should render button with same gaps`, () => {
initializeComponent({ slotPrefix: true, slotSuffix: true, slotDefault: true });
describe('when suffix, content and prefix', () => {
it(`should render button with same gaps`, () => {
initializeComponent({ slotPrefix: true, slotSuffix: true, slotDefault: true });

page().makeSnapshot();
});
page().makeSnapshot();
});
});

describe('when prefix and content', () => {
it(`should render button with prefix and content with correct gaps`, () => {
initializeComponent({ slotPrefix: true, slotDefault: true });
describe('when prefix and content', () => {
it(`should render button with prefix and content with correct gaps`, () => {
initializeComponent({ slotPrefix: true, slotDefault: true });

page().makeSnapshot();
});
page().makeSnapshot();
});
});

describe('when content and suffix', () => {
it(`should render button with content and suffix with correct gaps`, () => {
initializeComponent({ slotSuffix: true, slotDefault: true });
describe('when content and suffix', () => {
it(`should render button with content and suffix with correct gaps`, () => {
initializeComponent({ slotSuffix: true, slotDefault: true });

page().makeSnapshot();
});
page().makeSnapshot();
});
});

describe('when tag is different than button', () => {
it(`should render tag without type attribute`, () => {
initializeComponent({ tag: 'div' });

page().doesNotHaveType().makeSnapshot();
});
});
});
10 changes: 10 additions & 0 deletions packages/tests/components/SfListItem/SfListItem.PageObject.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,21 @@ export default class SfListItemObject extends BasePage {
return this;
}

hasDisabledPrefix() {
this.container.children().first().should('have.class', 'text-disabled-500');
return this;
}

hasSuffix() {
this.container.children().last().should('have.class', 'text-neutral-500');
return this;
}

hasDisabledSuffix() {
this.container.children().last().should('have.class', 'text-disabled-500');
return this;
}

isDisabled() {
this.container.should('have.class', 'cursor-not-allowed pointer-events-none text-disabled-900');
return this;
Expand Down
Loading

0 comments on commit 727a192

Please sign in to comment.