From 4cdb8e38b8eaf3a7bd6fecfb9fb322e6dbdab78a Mon Sep 17 00:00:00 2001 From: chelentos Date: Wed, 13 Sep 2023 14:23:48 +0300 Subject: [PATCH] fix: review fixes --- src/components/Toc/Toc.scss | 15 +++-- src/components/Toc/Toc.tsx | 65 +++++++++++-------- src/components/Toc/TocItem/TocItem.scss | 2 +- src/components/Toc/TocItem/TocItem.tsx | 34 ++++++---- .../Toc/__stories__/Toc.stories.scss | 2 +- .../Toc/__stories__/Toc.stories.tsx | 57 ++++++++++++++-- src/components/Toc/__tests__/Toc.test.tsx | 54 ++++++++++----- src/components/Toc/types.ts | 10 ++- 8 files changed, 170 insertions(+), 69 deletions(-) diff --git a/src/components/Toc/Toc.scss b/src/components/Toc/Toc.scss index 128e7e3b7b..ddb5470104 100644 --- a/src/components/Toc/Toc.scss +++ b/src/components/Toc/Toc.scss @@ -1,17 +1,24 @@ @use '../variables'; +@use '../../../styles/mixins.scss'; -$block: '.#{variables.$ns}toc'; +$block: '.#{variables.$ns-new}toc'; #{$block} { &__title { - font-size: var(--g-text-body-2-font-size); - font-weight: 500; + @include mixins.text-body-2(); + color: var(--g-color-text-primary); margin-bottom: 12px; } - &__sections { + &__sections, + &__subsections { + padding: 0; + margin: 0; + overflow-y: auto; overflow-x: hidden; + + list-style: none; } } diff --git a/src/components/Toc/Toc.tsx b/src/components/Toc/Toc.tsx index b23c8c942d..b97e0492ef 100644 --- a/src/components/Toc/Toc.tsx +++ b/src/components/Toc/Toc.tsx @@ -1,51 +1,62 @@ import React from 'react'; import type {QAProps} from '../types'; -import {block} from '../utils/cn'; +import {blockNew} from '../utils/cn'; import {TocItem} from './TocItem/TocItem'; -import type {TocItem as TocItemType} from './types'; +import type {TocItems} from './types'; import './Toc.scss'; -const b = block('toc'); +const b = blockNew('toc'); export interface TocProps extends QAProps { className?: string; - value: string; - onUpdate: (value: string) => void; - items: (TocItemType & { - items?: TocItemType[]; - })[]; + value?: string; + onUpdate?: (value: string) => void; + items: TocItems; } -export const Toc = React.forwardRef(function Toc(props, ref) { +export const Toc = React.forwardRef(function Toc(props, ref) { const {value: activeValue, items, className, onUpdate, qa} = props; return ( -
-
- {items.map(({value, title, items: childrenItems}) => ( - +
-
+ + ); }); diff --git a/src/components/Toc/TocItem/TocItem.scss b/src/components/Toc/TocItem/TocItem.scss index 6c95708da6..8ddfda3282 100644 --- a/src/components/Toc/TocItem/TocItem.scss +++ b/src/components/Toc/TocItem/TocItem.scss @@ -1,6 +1,6 @@ @use '../../variables'; -$block: '.#{variables.$ns}toc-item'; +$block: '.#{variables.$ns-new}toc-item'; #{$block} { $class: &; diff --git a/src/components/Toc/TocItem/TocItem.tsx b/src/components/Toc/TocItem/TocItem.tsx index 98d350d883..f6b8406d0f 100644 --- a/src/components/Toc/TocItem/TocItem.tsx +++ b/src/components/Toc/TocItem/TocItem.tsx @@ -1,38 +1,48 @@ import React from 'react'; -import {block} from '../../utils/cn'; +import {blockNew} from '../../utils/cn'; import {useActionHandlers} from '../../utils/useActionHandlers'; import type {TocItem as TocItemType} from '../types'; import './TocItem.scss'; -const b = block('toc-item'); +const b = blockNew('toc-item'); export interface TocItemProps extends TocItemType { childItem?: boolean; active?: boolean; - onClick: (value: string) => void; + onClick?: (value: string) => void; } export const TocItem = (props: TocItemProps) => { - const {childItem = false, active = false, onClick, title, value} = props; + const {active = false, childItem = false, content, href, value, onClick} = props; - const handleClick = () => onClick(value); + const handleClick = React.useCallback(() => { + if (value === undefined || !onClick) { + return; + } + + onClick(value); + }, [onClick, value]); const {onKeyDown} = useActionHandlers(handleClick); - return ( -
+ const item = + href === undefined ? (
- {title} + {content}
-
- ); + ) : ( + + {content} + + ); + + return
{item}
; }; diff --git a/src/components/Toc/__stories__/Toc.stories.scss b/src/components/Toc/__stories__/Toc.stories.scss index 3ce814ee1e..af9551ecce 100644 --- a/src/components/Toc/__stories__/Toc.stories.scss +++ b/src/components/Toc/__stories__/Toc.stories.scss @@ -1,6 +1,6 @@ @use '../../variables'; -$block: '.#{variables.$ns}toc-stories'; +$block: '.#{variables.$ns-new}toc-stories'; #{$block} { $class: &; diff --git a/src/components/Toc/__stories__/Toc.stories.tsx b/src/components/Toc/__stories__/Toc.stories.tsx index 5ee29c6460..57a58c9ced 100644 --- a/src/components/Toc/__stories__/Toc.stories.tsx +++ b/src/components/Toc/__stories__/Toc.stories.tsx @@ -26,29 +26,74 @@ Default.args = { items: [ { value: 'vm', - title: 'Virtual machine creation', + content: 'Virtual machine creation', }, { value: 'info', - title: 'Getting information about a group of virtual machines', + content: 'Getting information about a group of virtual machines', }, { value: 'disk', - title: 'Disk', + content: 'Disk', items: [ { value: 'control', - title: 'Disk controls', + content: 'Disk controls', }, { value: 'snapshots', - title: 'Disk snapshots', + content: 'Disk snapshots', }, ], }, { value: 'images', - title: 'Images with preinstalled software', + content: 'Images with preinstalled software', + }, + ], + className: b(), +}; + +const WithLinksTemplate: StoryFn = (args) => { + const [active, setActive] = React.useState('control'); + + return setActive(value)} />; +}; + +export const WithLinks = WithLinksTemplate.bind({}); +WithLinks.args = { + items: [ + { + value: 'vm', + content: 'Virtual machine creation', + href: '#vm', + }, + { + value: 'info', + content: 'Getting information about a group of virtual machines', + href: '#info', + }, + { + value: 'disk', + content: 'Disk', + href: '#disk', + items: [ + { + value: 'control', + content: 'Disk controls', + href: '#control', + }, + { + value: 'snapshots', + content: 'Disk snapshots', + href: '#snapshots', + }, + ], + }, + { + value: 'images', + content: 'Images with preinstalled software', + href: '#images', }, ], className: b(), diff --git a/src/components/Toc/__tests__/Toc.test.tsx b/src/components/Toc/__tests__/Toc.test.tsx index 96a7bf37f1..0b6460e059 100644 --- a/src/components/Toc/__tests__/Toc.test.tsx +++ b/src/components/Toc/__tests__/Toc.test.tsx @@ -8,57 +8,66 @@ import {Toc} from '../Toc'; const defaultItems = [ { value: 'firstItem', - title: 'First item', + content: 'First item', items: [], }, { value: 'secondItem', - title: 'Second item', + content: 'Second item', items: [], }, { value: 'thirdItem', - title: 'Third item', + content: 'Third item', items: [ { value: 'firstChildItem', - title: 'First child item', + content: 'First child item', + items: [], }, { value: 'secondChildItem', - title: 'Second child item', + content: 'Second child item', + items: [], }, ], }, { value: 'fourthItem', - title: 'Fourth item', + content: 'Fourth item', items: [], }, ]; const defaultValue = defaultItems[2].items[0].value; -const defaultTitle = defaultItems[2].items[0].title; + +const itemsWithLinks = defaultItems.map((item) => ({...item, href: `#${item.value}`})); + +const defaultValueWithLink = itemsWithLinks[2].items[0].value; const qaId = 'toc-component'; describe('Toc', () => { - test('renders active item correctly', () => { + test('calls onUpdate with correct value', async () => { + const nextValue = defaultItems[0].value; + const nextTitle = defaultItems[0].content; const onUpdateFn = jest.fn(); + const user = userEvent.setup(); render(); - const activeItem = screen.getByText(defaultTitle); + const nextItem = screen.getByText(nextTitle); + await user.click(nextItem); - expect(activeItem).toHaveAttribute('aria-checked', 'true'); + expect(onUpdateFn).toBeCalledWith(nextValue); }); - test('calls onUpdate with correct value', async () => { - const nextValue = defaultItems[0].value; - const nextTitle = defaultItems[0].title; + test('calls onUpdate with correct item with link', async () => { + const nextValue = itemsWithLinks[0].value; + const nextTitle = itemsWithLinks[0].content; const onUpdateFn = jest.fn(); const user = userEvent.setup(); - render(); + render(); const nextItem = screen.getByText(nextTitle); await user.click(nextItem); @@ -66,7 +75,7 @@ describe('Toc', () => { }); test('accessible for keyboard', async () => { - const firstTitle = defaultItems[0].title; + const firstTitle = defaultItems[0].content; const secondValue = defaultItems[1].value; const onUpdateFn = jest.fn(); const user = userEvent.setup(); @@ -80,6 +89,21 @@ describe('Toc', () => { expect(onUpdateFn).toBeCalledWith(secondValue); }); + test('accessible for keyboard with links', async () => { + const firstTitle = itemsWithLinks[0].content; + const secondValue = itemsWithLinks[1].value; + const onUpdateFn = jest.fn(); + const user = userEvent.setup(); + + render(); + const firstItem = screen.getByText(firstTitle); + await user.click(firstItem); + await user.tab(); + await user.keyboard('{Enter}'); + + expect(onUpdateFn).toBeCalledWith(secondValue); + }); + test('add className', () => { const className = 'my-class'; const onUpdateFn = jest.fn(); diff --git a/src/components/Toc/types.ts b/src/components/Toc/types.ts index 1128c94083..9a224a504d 100644 --- a/src/components/Toc/types.ts +++ b/src/components/Toc/types.ts @@ -1,5 +1,9 @@ export interface TocItem { - value: string; - title?: React.ReactNode; - selector?: string; + value?: string; + content?: React.ReactNode; + href?: string; } + +export type TocItems = (TocItem & { + items?: TocItem[]; +})[];