From 29037ec4960777e1504c06a9e2073d780bdfd9c8 Mon Sep 17 00:00:00 2001 From: LakeVostok Date: Wed, 22 May 2024 22:26:45 +0300 Subject: [PATCH 1/4] chore(Link): change codeowner (#1602) --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index 2dd3d5b98a..c85f4dc1e7 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -18,7 +18,7 @@ /src/components/Hotkey @d3m1d0v /src/components/Icon @amje /src/components/Label @goshander -/src/components/Link @LakeVostok +/src/components/Link @Estasie /src/components/List @korvin89 /src/components/Loader @SeqviriouM /src/components/Menu @NikitaCG From 9b8f4f897bfe1919f165029ea7865654f808367f Mon Sep 17 00:00:00 2001 From: Isaev Alexandr Date: Thu, 23 May 2024 15:56:30 +0300 Subject: [PATCH 2/4] fix(Flex,ListItemView): return ref drilling by React.forwardRef (#1612) --- src/components/layout/Flex/Flex.tsx | 8 +- .../components/ListItemView/ListItemView.tsx | 229 +++++++++--------- 2 files changed, 123 insertions(+), 114 deletions(-) diff --git a/src/components/layout/Flex/Flex.tsx b/src/components/layout/Flex/Flex.tsx index 22ca7d3c2c..3a93a16493 100644 --- a/src/components/layout/Flex/Flex.tsx +++ b/src/components/layout/Flex/Flex.tsx @@ -128,7 +128,10 @@ type FlexPropsWithTypedAttrs = FlexProps & * --- * Storybook - https://preview.gravity-ui.com/uikit/?path=/docs/layout--playground#flex */ -export const Flex = function Flex(props: FlexProps) { +export const Flex = React.forwardRef(function Flex( + props: FlexProps, + ref: FlexRef, +) { const { as: propsAs, direction, @@ -187,6 +190,7 @@ export const Flex = function Flex(props: Fl }, className, )} + ref={ref} style={{ flexDirection: applyMediaProps(direction), flexGrow: grow === true ? 1 : grow, @@ -213,6 +217,6 @@ export const Flex = function Flex(props: Fl : children} ); -} as (( +}) as (( props: FlexPropsWithTypedAttrs & {ref?: FlexRef}, ) => React.ReactElement) & {displayName: string}; diff --git a/src/components/useList/components/ListItemView/ListItemView.tsx b/src/components/useList/components/ListItemView/ListItemView.tsx index aff283be05..49d647a137 100644 --- a/src/components/useList/components/ListItemView/ListItemView.tsx +++ b/src/components/useList/components/ListItemView/ListItemView.tsx @@ -15,11 +15,13 @@ import './ListItemView.scss'; const b = block('list-item-view'); -export interface ListItemViewProps extends QAProps, ListItemCommonProps { +export interface ListItemViewProps + extends QAProps, + ListItemCommonProps { /** * Ability to override default html tag */ - as?: keyof JSX.IntrinsicElements; + as?: T; /** * @default `m` */ @@ -43,7 +45,7 @@ export interface ListItemViewProps extends QAProps, ListItemCommonProps { /** * Note: if passed and `disabled` option is `true` click will not be appear */ - onClick?(): void; + onClick?: React.ComponentPropsWithoutRef['onClick']; style?: React.CSSProperties; className?: string; role?: React.AriaRole; @@ -59,6 +61,11 @@ export interface ListItemViewProps extends QAProps, ListItemCommonProps { id: ListItemId; } +type ListItemViewRef = React.ComponentPropsWithRef['ref']; + +type ListItemViewPropsWithTypedAttrs = ListItemViewProps & + Omit, keyof ListItemViewProps>; + interface SlotProps extends FlexProps { indentation?: number; } @@ -85,118 +92,116 @@ const renderSafeIndentation = (indentation?: number) => { return null; }; -export const ListItemView = React.forwardRef( - ( - { - id, - as = 'div', - size = 'm', - active, - selected, - disabled, - activeOnHover: propsActiveOnHover, - className, - hasSelectionIcon = true, - indentation, - startSlot, - subtitle, - endSlot, - title, - height, - expanded, - dragging, - style, - role = 'option', - onClick: _onClick, - ...rest - }: ListItemViewProps, - ref?: any, - ) => { - const isGroup = typeof expanded === 'boolean'; - const onClick = disabled ? undefined : _onClick; - const activeOnHover = - typeof propsActiveOnHover === 'boolean' ? propsActiveOnHover : Boolean(onClick); +export const ListItemView = React.forwardRef(function ListItemView< + T extends React.ElementType = 'div', +>( + { + id, + as: asProps, + size = 'm', + active, + selected, + disabled, + activeOnHover: propsActiveOnHover, + className, + hasSelectionIcon = true, + indentation, + startSlot, + subtitle, + endSlot, + title, + height, + expanded, + dragging, + style, + role = 'option', + onClick: _onClick, + ...rest + }: ListItemViewPropsWithTypedAttrs, + ref?: ListItemViewRef, +) { + const as: React.ElementType = asProps || 'div'; + const isGroup = typeof expanded === 'boolean'; + const onClick = disabled ? undefined : _onClick; + const activeOnHover = + typeof propsActiveOnHover === 'boolean' ? propsActiveOnHover : Boolean(onClick); - return ( - + + {hasSelectionIcon && ( + + {selected ? ( + + ) : null} + )} - style={{ - minHeight: height ?? modToHeight[size][Number(Boolean(subtitle))], - ...style, - }} - as={as} - ref={ref} - alignItems="center" - gap="4" - justifyContent="space-between" - {...rest} - > - - {hasSelectionIcon && ( - + ) : null} + + {startSlot} + +
+ {typeof title === 'string' ? ( + - {selected ? ( - - ) : null} - + {title} + + ) : ( + title )} - - {renderSafeIndentation(indentation)} - - {isGroup ? ( - - ) : null} - - {startSlot} - -
- {typeof title === 'string' ? ( - - {title} - - ) : ( - title - )} - {typeof subtitle === 'string' ? ( - - {subtitle} - - ) : ( - subtitle - )} -
- - - {endSlot} + {typeof subtitle === 'string' ? ( + + {subtitle} + + ) : ( + subtitle + )} +
- ); - }, -); -ListItemView.displayName = 'ListItemView'; + {endSlot} + + ); +}) as ({ + ref, + ...props +}: ListItemViewPropsWithTypedAttrs & {ref?: ListItemViewRef}) => React.ReactElement; From 27b62e1fafdee78af4309f15f8ffc7b450ff78b6 Mon Sep 17 00:00:00 2001 From: itwillwork <15855766+itwillwork@users.noreply.github.com> Date: Thu, 23 May 2024 20:23:19 +0300 Subject: [PATCH 3/4] refactor: add prefer-ts-expect-error rule (#1608) --- .eslintrc | 1 + src/components/Disclosure/Disclosure.tsx | 2 +- .../Table/__tests__/Table.withTableSettings.test.tsx | 6 ++---- .../Table/hoc/withTableActions/withTableActions.tsx | 2 +- src/components/Table/hoc/withTableCopy/withTableCopy.tsx | 2 +- .../Table/hoc/withTableSelection/withTableSelection.tsx | 4 ++-- src/components/layout/demo/ColPresenter/ColPresenter.tsx | 2 +- test-utils/setup-tests.ts | 2 +- 8 files changed, 10 insertions(+), 11 deletions(-) diff --git a/.eslintrc b/.eslintrc index 2f52429610..f11af1b6e1 100644 --- a/.eslintrc +++ b/.eslintrc @@ -30,6 +30,7 @@ "jsx-a11y/no-autofocus": "off", "import/no-extraneous-dependencies": "off", "import/consistent-type-specifier-style": ["error", "prefer-top-level"], + "@typescript-eslint/prefer-ts-expect-error": "error", "@typescript-eslint/consistent-type-imports": ["error", {"prefer": "type-imports", "fixStyle": "separate-type-imports"}] }, "overrides": [ diff --git a/src/components/Disclosure/Disclosure.tsx b/src/components/Disclosure/Disclosure.tsx index 1d4a074b62..7353f0513f 100644 --- a/src/components/Disclosure/Disclosure.tsx +++ b/src/components/Disclosure/Disclosure.tsx @@ -42,7 +42,7 @@ export interface DisclosureProps extends QAProps { const isDisclosureSummaryComponent = isOfType(DisclosureSummary); -// @ts-ignore this ts-error is appears when forwarding ref. It complains that DisclosureComposition props is not provided initially +// @ts-expect-error this ts-error is appears when forwarding ref. It complains that DisclosureComposition props is not provided initially export const Disclosure: React.FunctionComponent & DisclosureComposition = React.forwardRef(function Disclosure(props, ref) { const { diff --git a/src/components/Table/__tests__/Table.withTableSettings.test.tsx b/src/components/Table/__tests__/Table.withTableSettings.test.tsx index 1e0f453d9e..9b84eea2b4 100644 --- a/src/components/Table/__tests__/Table.withTableSettings.test.tsx +++ b/src/components/Table/__tests__/Table.withTableSettings.test.tsx @@ -149,10 +149,8 @@ describe('withTableSettings', () => { ); expect(osSettings).toBeDefined(); expect(osxSettings).toBeDefined(); - // @ts-ignore - expect(osSettings.isSelected).toBe(true); - // @ts-ignore - expect(osxSettings.isSelected).toBe(false); + expect(osSettings?.isSelected).toBe(true); + expect(osxSettings?.isSelected).toBe(false); }); it('should return columns when no settings provided', () => { diff --git a/src/components/Table/hoc/withTableActions/withTableActions.tsx b/src/components/Table/hoc/withTableActions/withTableActions.tsx index cdb2f3145b..50e342e056 100644 --- a/src/components/Table/hoc/withTableActions/withTableActions.tsx +++ b/src/components/Table/hoc/withTableActions/withTableActions.tsx @@ -272,7 +272,7 @@ export function withTableActions( return (item: I, index: number, event: React.MouseEvent) => { if ( - // @ts-ignore + // @ts-expect-error event.nativeEvent.target.matches( `.${actionsButtonCn}, .${actionsButtonCn} *`, ) diff --git a/src/components/Table/hoc/withTableCopy/withTableCopy.tsx b/src/components/Table/hoc/withTableCopy/withTableCopy.tsx index 966f009928..b9efad1f2e 100644 --- a/src/components/Table/hoc/withTableCopy/withTableCopy.tsx +++ b/src/components/Table/hoc/withTableCopy/withTableCopy.tsx @@ -104,7 +104,7 @@ export function withTableCopy( return (item: I, index: number, event: React.MouseEvent) => { const buttonClassName = b('copy-button'); if ( - // @ts-ignore + // @ts-expect-error event.nativeEvent.target.matches( `.${buttonClassName}, .${buttonClassName} *`, ) diff --git a/src/components/Table/hoc/withTableSelection/withTableSelection.tsx b/src/components/Table/hoc/withTableSelection/withTableSelection.tsx index cfa4c89b30..497de15a6f 100644 --- a/src/components/Table/hoc/withTableSelection/withTableSelection.tsx +++ b/src/components/Table/hoc/withTableSelection/withTableSelection.tsx @@ -120,7 +120,7 @@ export function withTableSelection( event: React.ChangeEvent, ) => { const {checked} = event.target; - // @ts-ignore shiftKey is defined for click events + // @ts-expect-error shiftKey is defined for click events const isShiftPressed = event.nativeEvent.shiftKey; const {data, selectedIds, onSelectionChange} = this.props; @@ -191,7 +191,7 @@ export function withTableSelection( return (item: I, index: number, event: React.MouseEvent) => { const checkboxClassName = b('selection-checkbox'); if ( - // @ts-ignore + // @ts-expect-error event.nativeEvent.target.matches( `.${checkboxClassName}, .${checkboxClassName} *`, ) diff --git a/src/components/layout/demo/ColPresenter/ColPresenter.tsx b/src/components/layout/demo/ColPresenter/ColPresenter.tsx index c203cf7265..d77a5f2bbc 100644 --- a/src/components/layout/demo/ColPresenter/ColPresenter.tsx +++ b/src/components/layout/demo/ColPresenter/ColPresenter.tsx @@ -4,7 +4,7 @@ import {Col} from '../../Col/Col'; import type {ColProps} from '../../Col/Col'; import {Box} from '../Box/Box'; -// @ts-ignore-error +// @ts-expect-error const pickSizeProps = ({l, xl, s, m, xxl, xxxl, size}: T = {}): string => { // skip empty values return Object.entries({...{l, xl, s, m, xxl, xxxl, size}}) diff --git a/test-utils/setup-tests.ts b/test-utils/setup-tests.ts index 2d03f72af0..374470e6e5 100644 --- a/test-utils/setup-tests.ts +++ b/test-utils/setup-tests.ts @@ -20,7 +20,7 @@ global.ResizeObserver = class implements ResizeObserver { jest.mock( 'react-virtualized-auto-sizer', () => - //@ts-ignore + //@ts-expect-error ({children}) => children({height: 400, width: 400}), ); From e35ce8bf236b5399daf07b06a2e68d9c14dc44b5 Mon Sep 17 00:00:00 2001 From: zzman <43282255+GermanVor@users.noreply.github.com> Date: Fri, 24 May 2024 13:11:11 +0200 Subject: [PATCH 4/4] chore(Pagination): add tests (#1605) --- .../Pagination/__tests__/Pagination.test.tsx | 85 +++++++++++++++++++ .../PaginationPage/PaginationPage.tsx | 4 +- src/components/Pagination/constants.ts | 4 + 3 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 src/components/Pagination/__tests__/Pagination.test.tsx diff --git a/src/components/Pagination/__tests__/Pagination.test.tsx b/src/components/Pagination/__tests__/Pagination.test.tsx new file mode 100644 index 0000000000..9ccca13aa5 --- /dev/null +++ b/src/components/Pagination/__tests__/Pagination.test.tsx @@ -0,0 +1,85 @@ +import React from 'react'; + +import {noop} from 'lodash'; + +import {render, screen} from '../../../../test-utils/utils'; +import {Pagination} from '../Pagination'; +import {PaginationQa, getPaginationPageQa} from '../constants'; + +describe('Pagination component', () => { + test('Single page', () => { + render(); + + const firstButton = screen.getByTestId(PaginationQa.PaginationButtonFirst); + expect(firstButton).toBeDisabled(); + + const prevButton = screen.getByTestId(PaginationQa.PaginationButtonPrevious); + expect(prevButton).toBeDisabled(); + + const nextButton = screen.getByTestId(PaginationQa.PaginationButtonNext); + expect(nextButton).toBeDisabled(); + }); + + describe('Two pages', () => { + const PAGE_SIZE = 20; + const TOTAL = PAGE_SIZE + 1; + + const FIRST_PAGE = 1; + const SECOND_PAGE = 2; + + test('First page is current', () => { + render( + , + ); + + const firstPageButton = screen.getByTestId(getPaginationPageQa(FIRST_PAGE)); + expect(firstPageButton).toHaveAttribute('aria-pressed', 'true'); + + const secondPageButton = screen.getByTestId(getPaginationPageQa(SECOND_PAGE)); + expect(secondPageButton).toHaveAttribute('aria-pressed', 'false'); + + const firstButton = screen.getByTestId(PaginationQa.PaginationButtonFirst); + expect(firstButton).toBeDisabled(); + + const prevButton = screen.getByTestId(PaginationQa.PaginationButtonPrevious); + expect(prevButton).toBeDisabled(); + + const nextButton = screen.getByTestId(PaginationQa.PaginationButtonNext); + expect(nextButton).not.toBeDisabled(); + }); + + test('Second page is current', () => { + render( + , + ); + + const firstPageButton = screen.getByTestId(getPaginationPageQa(FIRST_PAGE)); + expect(firstPageButton).toHaveAttribute('aria-pressed', 'false'); + + const secondPageButton = screen.getByTestId(getPaginationPageQa(SECOND_PAGE)); + expect(secondPageButton).toHaveAttribute('aria-pressed', 'true'); + + const firstButton = screen.getByTestId(PaginationQa.PaginationButtonFirst); + expect(firstButton).not.toBeDisabled(); + + const prevButton = screen.getByTestId(PaginationQa.PaginationButtonPrevious); + expect(prevButton).not.toBeDisabled(); + + const nextButton = screen.getByTestId(PaginationQa.PaginationButtonNext); + expect(nextButton).toBeDisabled(); + }); + }); + + test('Total property undefined', () => { + render(); + + const nextButton = screen.getByTestId(PaginationQa.PaginationButtonNext); + + expect(nextButton).not.toBeDisabled(); + }); +}); diff --git a/src/components/Pagination/components/PaginationPage/PaginationPage.tsx b/src/components/Pagination/components/PaginationPage/PaginationPage.tsx index 9288fb824c..e55fc8fe8a 100644 --- a/src/components/Pagination/components/PaginationPage/PaginationPage.tsx +++ b/src/components/Pagination/components/PaginationPage/PaginationPage.tsx @@ -2,7 +2,7 @@ import React from 'react'; import {Button} from '../../../Button'; import {block} from '../../../utils/cn'; -import {PaginationQa} from '../../constants'; +import {getPaginationPageQa} from '../../constants'; import type {PageItem, PaginationProps, PaginationSize} from '../../types'; import './PaginationPage.scss'; @@ -18,7 +18,7 @@ type Props = { }; export const PaginationPage = ({item, size, pageSize, className, onUpdate}: Props) => { - const qa = `${PaginationQa.PaginationPage}-${item.page}`; + const qa = getPaginationPageQa(item.page); if (item.simple) { return (
diff --git a/src/components/Pagination/constants.ts b/src/components/Pagination/constants.ts index 005b4e602d..c61df8a4e3 100644 --- a/src/components/Pagination/constants.ts +++ b/src/components/Pagination/constants.ts @@ -7,3 +7,7 @@ export const PaginationQa = { PaginationButtonPrevious: 'pagination-button-previous', PaginationButtonNext: 'pagination-button-next', }; + +export const getPaginationPageQa = (pageNumber: number) => { + return `${PaginationQa.PaginationPage}-${pageNumber}`; +};