fix(ImageBaseOverlay): fix nested Tappable #7006
Conversation
github-actions
bot
added
the
patch
size-limit report 📦
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
e2e tests |
👀 Docs deployed
Commit 72d4bcf |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7006 +/- ##
==========================================
+ Coverage 83.49% 83.65% +0.16%
==========================================
Files 351 352 +1
Lines 10501 10575 +74
Branches 3477 3501 +24
==========================================
+ Hits 8768 8847 +79
+ Misses 1733 1728 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
vkcom-publisher
added
pr-needs-work
BlackySoul
force-pushed
the
fix/7001/ImageBase.overlay_fix_nested_tappable
branch
from
737557d
to
7ac884e
Compare
packages/vkui/src/components/ImageBase/ImageBaseOverlay/ImageBaseOverlay.test.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrey Medvedev <[email protected]>
inomdzhon
removed
the
patch
| * > использовали иконку. | ||
| */ | ||
| children: React.ReactElement<ImageBaseExpectedIconProps>; | ||
| nonInteractive?: false; |
There was a problem hiding this comment.
а можем на disableInteractive переименовать? 👉 👈 у нас сейчас в FloatingComponentProps так называется
только на уровне внешнего API, хук useNonInteractiveOverlayProps можно так и оставить
There was a problem hiding this comment.
только вот юниты придётся удалить на добавление класса...
| }; | ||
| } | ||
|
|
||
| export function useHover() { |
There was a problem hiding this comment.
можем при on-hover в CSS перенести на @media (--hover-has)?
| overlayShown: visibility === 'always' || (visibility === 'on-hover' && hovered), | ||
| }; | ||
|
|
||
| if (restProps.nonInteractive) { |
There was a problem hiding this comment.
вижу nonInteractive не нужон в ImageBaseOverlayNonInteractive и ImageBaseOverlayInteractive, можно вытащить из пропсов, а в типах ImageBaseOverlayNonInteractive и ImageBaseOverlayInteractive исключить проп через Omit
Описание
by designвложенныеTappableресетятhover/active-состояние верхнеуровневогоTappable, чтобы поддержать следующее поведение (актуально только для 2-х уровневой вложенности, это оке?):2024-06-11.00.58.07.mov
В случае с
ImageBaseOverlayтакое поведение нежелательно.Изменения
Добавила
DisableClickableLockStateContextдля отключения такого поведения (принимаю в пожертвование лучшее название).Сначала думала сделать это через проп на
Tappable, но, кажется, что DX такого решения будет сомнительный. Например, в случае сImageBaseOverlayмы точно знаем, что дефолтное поведение не нужно, но будем заставлять пользователя в каждую кнопку прокидывать проп 🤔Изменения v2
В текущей версии компонента некорректно передавать в
overlayинтерактивные элементы, потому что получается кнопка в кнопке.В новой реализации убрала использование
Tappable- кажется, это неbreaking change, потому что мы никакие пропы, специфичные дляTappableне прокидывали - по сути он нам нужен только дляhover-эффекта (остальное мы либо отключаем, либо не можем отключить, но оно мешает - например, меняется вид поинтера). В итоге получился интерактивный и неинтерактивный компоненты, принимаем решение, какой из них рисовать, на основе нового пропаnonInteractive- в таске предлагали завязываться наonClick, но такое мы не можем провернуть - потому что сейчас поведение по-умолчанию - если onClick не передан, то мы подставляемnoopиhover-эффект работает из коробки (кому-то можем сломать это поведение).