-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/pure cell different clickable areas #1177
Conversation
🦋 Changeset detectedLatest commit: 2b48e69 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Pull Request Test Coverage Report for Build 8859001993Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Собрана новая демка. |
Нужно добавить changeset |
packages/pure-cell/src/component.tsx
Outdated
|
||
const setHover = () => setHoverState(true); | ||
const unsetHover = () => setHoverState(false); | ||
const setActive = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь также можно написать в одну строку
const setActive = () => setActiveState(true);
}) => { | ||
const pureCellContext = useContext(PureCellContext); | ||
|
||
const Component = onClick ? 'button' : 'section'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Строковые литералы ('button', 'section'
) не должны быть разбросаны по документу, а объявлены в какой либо сущности (enum, object, map). В других файлах поправить аналогичным образом.
packages/pure-cell/src/component.tsx
Outdated
@@ -120,11 +123,37 @@ const PureCellComponent = forwardRef<HTMLElement, PureCellProps>( | |||
) => { | |||
const cellRef = useRef<HTMLDivElement>(null); | |||
const [focused] = useFocus(cellRef, 'keyboard'); | |||
const [hoverState, setHoverState] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Желательно указывать дженерики useState<boolean>
}) => { | ||
const pureCellContext = useContext(PureCellContext); | ||
|
||
const Component = onClick ? 'button' : 'section'; | ||
|
||
const events = onClick |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут можно убрать проверку, и сделать объект onClickEvents. А саму проверку делать уже в spread операторе {...(onClick && { onClickEvents })}
Подобное встречается несколько раз по коду, в целом на твое усмотрение. Можно оставить и так, но если количество эвентов в будущем изменится придется переписать тернарник.
) => { | ||
( | ||
onMouseEnter as | ||
| React.MouseEventHandler<HTMLAnchorElement> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Дублирование типов, React.MouseEventHandler<HTMLAnchorElement> React.MouseEventHandler<HTMLButtonElement>
. Может быть вынесем в рамках файла?
а можно изолировать поведение с опрозрачниванием при ховере и нажатии? В темах отличных от АО это не нужно. Если бы прозрачность изменялась только в теме клик — было-бы топчик |
Собрана новая демка. |
Вынес в переменные в тему клика опасити для hover/active |
Собрана новая демка. |
Собрана новая демка. |
.changeset/khaki-bees-do.md
Outdated
|
||
Добавлена пропс onClick для компонента PureCell.Graphics | ||
Добавлена пропс onClick для компонента PureCell.Main | ||
Добавлена пропс onClick для компонента PureCell.Addon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нужно добавить дефис в начало каждой строки
https://core-ds.github.io/core-components/master/?path=/docs/instructions-contributing--docs
Собрана новая демка. |
Опишите проблему
Необходимо было добавить разные кликабельные области для PureCell.
Дизайн https://www.figma.com/file/xsWJdFDoAyA8VkbuMxP4yR/Web-%3A%3A-Core-Wrappers?type=design&node-id=0-1&mode=design&t=vQhXj8io04PKwbws-0
Шаги для воспроизведения
Добавить onClick для Graphic Main Addon FooterButton и PureCelll
Ожидаемое поведение
При нажатии на кликабельную область стработает только обрабочик этой области ( при наличии пропса onClick у PureCell ), hover и active состояния воспроизводятся только у кликабельной области. При взаимодействии со всей PureCell ячейкой, hover и active состояния воспроизводятся на всей ячейке ( при наличии пропса onClick у PureCell )