fix(ImageBaseOverlay): fix nested Tappable #7006
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 65c988e |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7006 +/- ##
==========================================
+ Coverage 83.91% 83.93% +0.02%
==========================================
Files 357 358 +1
Lines 10785 10814 +29
Branches 3557 3560 +3
==========================================
+ Hits 9050 9077 +27
- Misses 1735 1737 +2
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
mendrew
previously approved these changes
Jun 27, 2024
packages/vkui/src/components/ImageBase/ImageBaseOverlay/ImageBaseOverlay.test.tsx
Outdated
Show resolved
Hide resolved
mendrew
previously approved these changes
Jun 27, 2024
inomdzhon
removed
the
patch
inomdzhon
reviewed
Jun 27, 2024
packages/vkui/src/components/ImageBase/ImageBaseOverlay/types.ts
Outdated
Show resolved
Hide resolved
packages/vkui/src/components/ImageBase/ImageBaseOverlay/hooks.ts
Outdated
Show resolved
Hide resolved
packages/vkui/src/components/ImageBase/ImageBaseOverlay/ImageBaseOverlay.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrey Medvedev <andr.medv.spb@gmail.com>
BlackySoul
force-pushed
the
fix/7001/ImageBase.overlay_fix_nested_tappable
branch
from
72d4bcf
to
3e63cb6
Compare
SevereCloud
approved these changes
Jul 8, 2024
BlackySoul
added
the
patch
BlackySoul
added a commit
that referenced
this pull request
Jul 8, 2024
* fix(ImageBaseOverlay): fix nested Tappable * fix(ImageBaseOverlay): add non interactive wrapper * chore: clean up Co-authored-by: Andrey Medvedev <andr.medv.spb@gmail.com> * chore: fix prettier * fix: review notes * fix merge * fix: remove tests * fix: revert DisableClickableLockStateContext adding --------- Co-authored-by: Andrey Medvedev <andr.medv.spb@gmail.com>
BlackySoul
added a commit
that referenced
this pull request
Jul 8, 2024
* fix(ImageBaseOverlay): fix nested Tappable * fix(ImageBaseOverlay): add non interactive wrapper * chore: clean up Co-authored-by: Andrey Medvedev <andr.medv.spb@gmail.com> * chore: fix prettier * fix: review notes * fix merge * fix: remove tests * fix: revert DisableClickableLockStateContext adding --------- Co-authored-by: Andrey Medvedev <andr.medv.spb@gmail.com>
BlackySoul
added a commit
that referenced
this pull request
Jul 8, 2024
) * fix(ImageBaseOverlay): fix nested Tappable (#7006) * fix(ImageBaseOverlay): fix nested Tappable * fix(ImageBaseOverlay): add non interactive wrapper * chore: clean up Co-authored-by: Andrey Medvedev <andr.medv.spb@gmail.com> * chore: fix prettier * fix: review notes * fix merge * fix: remove tests * fix: revert DisableClickableLockStateContext adding --------- Co-authored-by: Andrey Medvedev <andr.medv.spb@gmail.com> * fix: run prettier * CHORE: Update screenshots * fix: delete e2e test --------- Co-authored-by: Andrey Medvedev <andr.medv.spb@gmail.com> Co-authored-by: GitHub Action <actions@github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Описание
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-эффекта (остальное мы либо отключаем, либо не можем отключить, но оно мешает - например, меняется вид поинтера). В итоге получился интерактивный и неинтерактивный компоненты, принимаем решение, какой из них рисовать, на основе нового пропаdisableInteractive- в таске предлагали завязываться наonClick, но такое мы не можем провернуть - потому что сейчас поведение по-умолчанию - если onClick не передан, то мы подставляемnoopиhover-эффект работает из коробки (кому-то можем сломать это поведение).