-
Notifications
You must be signed in to change notification settings - Fork 197
feat: add CustomScrollView.Tint #9359
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
base: master
Are you sure you want to change the base?
Conversation
a85a9af to
886c55f
Compare
size-limit report 📦
|
e2e tests
|
👀 Docs deployed
📦 Package ✅yarn add @vkontakte/vkui@https://development.s3.prodcloud.vk.team/pull/9359/021b56a5b4776802865111dfc69af38c909d4a4f/pkg/@vkontakte/vkui/_pkg.tgzCommit 021b56a |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9359 +/- ##
==========================================
+ Coverage 94.73% 94.75% +0.02%
==========================================
Files 440 441 +1
Lines 11898 11930 +32
Branches 4386 4393 +7
==========================================
+ Hits 11271 11304 +33
+ Misses 627 626 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
EldarMuhamethanov
left a comment
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.
👍
packages/vkui/src/components/CustomScrollView/Tint/CustomScrollViewTint.tsx
Outdated
Show resolved
Hide resolved
| const onScroll = (e: React.UIEvent<HTMLDivElement>) => { | ||
| const target = e.currentTarget; | ||
| updateTint(target); | ||
| }; |
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.
Пересчет теней происходит только при скролле, но при изменении размеров контейнера не происходит пересчет. Может добавить useResizeObserver?
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.
Я тут подумал, размер контейнера CustomScrollView может и не измениться при изменении реального размера содержимого, так что тут useResizeObserver будет бесполезным. Тут либо добавлять еще один контейнер, у которого будет меняться размер, при изменении размеров содержимого, либо забить надо это и вовсе убрать useResizeObserver, но тогда могут появиться баги в будущем
| )} | ||
| {...restProps} | ||
| > | ||
| {children({ getRootRef: scrollRef, onScroll })} |
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.
У нас вроде не очень приветствуется использование renderFn подхода, потому что нужно мемоизировать children функцию снаружи и свойства прокидываемые в функцию. Может лучше использовать контекст как в Accordion.
9be823d to
a778ed1
Compare
698fda9 to
021b56a
Compare
Описание
Добавляем компонент для создания теней к прокручиваемой области
Release notes
Новые компоненты