Skip to content

Commit

Permalink
a11y(CardScroll): fix a11y, ul>li combination (#8140)
Browse files Browse the repository at this point in the history 
* a11y(CardScroll): fix a11y, ul>li combination

* feat(HorizontalScroll): add props to customize content wrapper

* fix: fix prettier

* fix: rename css variable and delete styles

* fix(CardScroll): revert gap

* fix(CardScroll): return realization with before/after

* fix(HorizontalScroll): fix type description
  • Loading branch information
@EldarMuhamethanov
EldarMuhamethanov authored Jan 28, 2025
1 parent 750edd0 commit a7b278b
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 61 deletions.
32 changes: 16 additions & 16 deletions packages/vkui/src/components/CardScroll/CardScroll.module.css
View file
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
.host {
margin: 0;
padding: 0;
list-style-type: none;
--vkui_internal--CardScroll_horizontal_padding: 0;
}

.in {
display: flex;
align-items: stretch;
inline-size: 100%;
margin: 0;
padding: 0;
list-style-type: none;
}

.gap {
display: block;
flex-shrink: 0;
.in::after,
.in::before {
content: '';
padding-inline: calc(var(--vkui_internal--CardScroll_horizontal_padding) / 2);
}

.host + .host {
margin-block-start: 16px;
}

.withPaddings .gap {
inline-size: var(--vkui--size_base_padding_horizontal--regular);
.withPaddings {
--vkui_internal--CardScroll_horizontal_padding: var(
--vkui--size_base_padding_horizontal--regular
);
}

/**
Expand All @@ -29,17 +31,15 @@
*/
/* stylelint-disable-next-line selector-pseudo-class-disallowed-list */
:global(.vkuiInternalSplitCol--viewWidth-tabletPlus):global(.vkuiInternalSplitCol--spaced-auto)
.withPaddings
.gap {
inline-size: 16px;
.withPaddings {
--vkui_internal--CardScroll_horizontal_padding: 16px;
}

@media (--viewWidth-smallTabletPlus) {
/* stylelint-disable-next-line selector-pseudo-class-disallowed-list */
:global(.vkuiInternalSplitCol--viewWidth-none):global(.vkuiInternalSplitCol--spaced-auto)
.withPaddings
.gap {
inline-size: 16px;
.withPaddings {
--vkui_internal--CardScroll_horizontal_padding: 16px;
}
}

Expand Down
52 changes: 28 additions & 24 deletions packages/vkui/src/components/CardScroll/CardScroll.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { type CSSProperties } from 'react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { baselineComponent } from '../../testing/utils';
import { CardScroll } from './CardScroll';
Expand Down Expand Up @@ -38,25 +37,35 @@ const mockCardScrollData = (container: HTMLElement, cardsCount: number, defaultS
jest.spyOn(card, 'offsetWidth', 'get').mockImplementation(() => width);
});

jest.spyOn(window, 'getComputedStyle').mockImplementation(() => {
const styles: CSSProperties = {
marginRight: '8px',
};
return styles as CSSStyleDeclaration;
});
const originalGetComputedStyle = window.getComputedStyle;

const getComputedStyleInstance = jest
.spyOn(window, 'getComputedStyle')
.mockImplementation((e) => {
return {
...originalGetComputedStyle(e),
marginRight: '8px',
getPropertyValue: (property: string) => {
if (property === '--vkui_internal--CardScroll_horizontal_padding') {
return '12px';
}
return '';
},
};
});

const cardScrollContainer = container.getElementsByClassName(styles.in)[0] as HTMLDivElement;
jest.spyOn(cardScrollContainer, 'offsetWidth', 'get').mockImplementation(() => 1009);

const gap = container.getElementsByClassName(styles.gap)[0] as HTMLDivElement;
jest.spyOn(gap, 'offsetWidth', 'get').mockImplementation(() => 12);

const horizontalScroll = container.getElementsByClassName(
horizontalScrollStyles.in,
)[0] as HTMLDivElement;
return {
horizontalScrollData: setupHorizontalScrollData(horizontalScroll, defaultScrollLeft),
horizontalScroll,
mocksRestore: () => {
[getComputedStyleInstance].forEach((mock) => mock.mockRestore());
},
};
};

Expand All @@ -74,7 +83,7 @@ const setup = ({ defaultScrollLeft = 50, cardsCount = 6 }: PrepareDataParams) =>
</CardScroll>,
);

const { horizontalScrollData, horizontalScroll } = mockCardScrollData(
const { horizontalScrollData, horizontalScroll, mocksRestore } = mockCardScrollData(
container,
cardsCount,
defaultScrollLeft,
Expand All @@ -84,23 +93,15 @@ const setup = ({ defaultScrollLeft = 50, cardsCount = 6 }: PrepareDataParams) =>

return {
horizontalScrollData,
mocksRestore,
};
};

describe('CardScroll', () => {
baselineComponent(CardScroll, {
a11yConfig: {
rules: {
// TODO [a11y]: "<ul> and <ol> must only directly contain <li>, <script> or <template> elements (list)"
// https://dequeuniversity.com/rules/axe/4.5/aria-required-parent?application=axeAPI
// see https://github.com/VKCOM/VKUI/issues/8135
list: { enabled: false },
},
},
});
baselineComponent(CardScroll);

it('check scroll by click arrow left', async () => {
const { horizontalScrollData } = setup({
const { horizontalScrollData, mocksRestore } = setup({
defaultScrollLeft: 1470,
});

Expand All @@ -120,10 +121,11 @@ describe('CardScroll', () => {
await waitFor(() => {
expect(horizontalScrollData.scrollLeft).toBe(0);
});
mocksRestore();
});

it('check scroll by click arrow right', async () => {
const { horizontalScrollData } = setup({});
const { horizontalScrollData, mocksRestore } = setup({});

const arrowRight = screen.getByTestId('ScrollArrowRight');

Expand All @@ -141,10 +143,11 @@ describe('CardScroll', () => {
await waitFor(() => {
expect(horizontalScrollData.scrollLeft).toBe(1477);
});
mocksRestore();
});

it('check scroll by click both arrows', async () => {
const { horizontalScrollData } = setup({});
const { horizontalScrollData, mocksRestore } = setup({});

const arrowRight = screen.getByTestId('ScrollArrowRight');
const arrowLeft = screen.getByTestId('ScrollArrowLeft');
Expand All @@ -158,5 +161,6 @@ describe('CardScroll', () => {
await waitFor(() => {
expect(horizontalScrollData.scrollLeft).toBe(0);
});
mocksRestore();
});
});
43 changes: 24 additions & 19 deletions packages/vkui/src/components/CardScroll/CardScroll.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export interface CardScrollProps
* Добавляет отступы по краям слева и справа
*/
padding?: boolean;
/**
* Позволяет поменять тег используемый для обертки над карточками
*/
CardsListComponent?: React.ElementType;
}

/**
Expand All @@ -36,18 +40,25 @@ export const CardScroll = ({
size = 's',
showArrows = true,
padding = false,
Component = 'ul',
CardsListComponent = 'ul',
prevButtonTestId,
nextButtonTestId,
...restProps
}: CardScrollProps): React.ReactNode => {
const refContainer = React.useRef<HTMLDivElement>(null);
const gapRef = React.useRef<HTMLDivElement>(null);

const { window } = useDOM();

const getPadding = (container: HTMLElement) => {
return parseFloat(
window!
.getComputedStyle(container)
.getPropertyValue('--vkui_internal--CardScroll_horizontal_padding'),
);
};

function getScrollToLeft(offset: number): number {
if (!refContainer.current || !gapRef.current) {
if (!refContainer.current) {
return offset;
}
const containerWidth = refContainer.current.offsetWidth;
Expand All @@ -64,24 +75,19 @@ export const CardScroll = ({
return offset;
}

if (slideIndex === 0) {
return 0;
}

const slide = refContainer.current.children[slideIndex] as HTMLElement;
const padding = getPadding(refContainer.current);
const scrollTo = slide.offsetLeft - (containerWidth - slide.offsetWidth) + padding;

const scrollTo =
slide.offsetLeft - (containerWidth - slide.offsetWidth) + gapRef.current.offsetWidth;

if (scrollTo <= 2 * gapRef.current.offsetWidth) {
if (scrollTo <= 2 * padding) {
return 0;
}

return scrollTo;
}

function getScrollToRight(offset: number): number {
if (!refContainer.current || !gapRef.current) {
if (!refContainer.current) {
return offset;
}

Expand All @@ -95,13 +101,13 @@ export const CardScroll = ({
return offset;
}

return slide.offsetLeft - gapRef.current.offsetWidth;
const padding = getPadding(refContainer.current);
return slide.offsetLeft - padding;
}

return (
<RootComponent
{...restProps}
Component={Component}
baseClassName={classNames(
styles.host,
'vkuiInternalCardScroll',
Expand All @@ -115,12 +121,11 @@ export const CardScroll = ({
showArrows={showArrows}
prevButtonTestId={prevButtonTestId}
nextButtonTestId={nextButtonTestId}
ContentWrapperComponent={CardsListComponent}
contentWrapperRef={refContainer}
contentWrapperClassName={styles.in}
>
<div className={styles.in} ref={refContainer}>
<span className={styles.gap} ref={gapRef} />
{children}
<span className={styles.gap} />
</div>
{children}
</HorizontalScroll>
</RootComponent>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/vkui/src/components/CardScroll/Readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

## Цифровая доступность (a11y)

- По умолчанию используется тег `"ul"` для повышения доступности компонента. Вы можете прокинуть необходимый вам тег через prop `Component`.
- Есть свойство `CardsListComponent`, позволяющее изменить тег для списка карточек. По умолчанию используется тег `"ul"` для повышения доступности компонента.
Ссылки на источники: [статья про доступность карточек](https://inclusive-components.design/cards/).

```jsx
Expand Down
23 changes: 22 additions & 1 deletion packages/vkui/src/components/HorizontalScroll/HorizontalScroll.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,18 @@ export interface HorizontalScrollProps
* Передает атрибут `data-testid` для кнопки прокрутки горизонтального скролла в направлении следующего элемента
*/
nextButtonTestId?: string;
/**
* Позволяет поменять тег используемый для обертки над контентом, прокинутым в `children`
*/
ContentWrapperComponent?: React.ElementType;
/**
* `ref` для обертки над контентом, прокинутым в `children`
*/
contentWrapperRef?: React.Ref<HTMLElement>;
/**
* Специфичный `className` для обертки над контентом, прокинутым в `children`
*/
contentWrapperClassName?: string;
}

/**
Expand Down Expand Up @@ -179,6 +191,10 @@ export const HorizontalScroll = ({
prevButtonTestId,
nextButtonTestId,
getRootRef,
// ContentWrapper
ContentWrapperComponent = 'div',
contentWrapperRef,
contentWrapperClassName,
...restProps
}: HorizontalScrollProps): React.ReactNode => {
const [canScrollLeft, setCanScrollLeft] = React.useState(false);
Expand Down Expand Up @@ -307,7 +323,12 @@ export const HorizontalScroll = ({
/>
)}
<div className={styles.in} ref={scrollerRef} onScroll={calculateArrowsVisibility}>
<div className={styles.inWrapper}>{children}</div>
<ContentWrapperComponent
className={classNames(styles.inWrapper, contentWrapperClassName)}
ref={contentWrapperRef}
>
{children}
</ContentWrapperComponent>
</div>
</RootComponent>
);
Expand Down

0 comments on commit a7b278b

Please sign in to comment.