[FRNT-495] implement accordion component #105
Conversation
Irinaristova
requested review from
tatinacher and
sergeysova
and removed request for
tatinacher
| if (document.activeElement?.tagName === LI_TAG) { | ||
| (document.activeElement?.nextElementSibling as HTMLElement)?.focus(); | ||
| } else { | ||
| (listNode?.firstChild as HTMLElement)?.focus(); | ||
| } |
There was a problem hiding this comment.
Вообще, очень плохо, что handler on key down полагается нас внутреннюю структуру списка. То есть фактически может быть использован только в одном месте — в списке
There was a problem hiding this comment.
Но наверное, это замечание не к этому ПР
| const AccordionBase: React.FC<AccordionProps & Variant> = ({ | ||
| children, | ||
| className, | ||
| isOpen = true, |
There was a problem hiding this comment.
| isOpen = true, | |
| isOpened = true, |
open это глагол, он не может описывать состояние. В русском аналогично, нельзя использовать "открыть" для описания состояния, мы используем специальную форму "открыто", в английском аналогично is opened
| <div className={className} data-variant={variant}> | ||
| <div | ||
| data-header | ||
| data-open={isContentVisible} |
There was a problem hiding this comment.
| data-open={isContentVisible} | |
| data-opened={isContentVisible} |
| }) => { | ||
| const [isContentVisible, setContentVisible] = React.useReducer((is) => !is, isOpen); | ||
|
|
||
| const onKeyDown = React.useCallback( |
There was a problem hiding this comment.
Аккордеон имеет много элементов, смотря на название этой переменной невозможно понять на какой элемент будет добавлен этот хендлер.
Вдруг глобально?
| const onKeyDown = React.useCallback( | |
| const onHeaderKeyDown = React.useCallback( |
There was a problem hiding this comment.
тогда наверно это правило лучше использовать для всех элементов, у которых может быть вложенность ?
There was a problem hiding this comment.
вопрос не во вложенности, а в качестве именования
No description provided.