-
Notifications
You must be signed in to change notification settings - Fork 184
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(Carousel): add Gallery with loop #5744
Conversation
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. Latest deployment of this branch, based on commit aa883d3:
|
size-limit report 📦
|
e2e tests |
👀 Docs deployed
Commit aa883d3 |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5744 +/- ##
==========================================
- Coverage 81.19% 79.16% -2.04%
==========================================
Files 299 303 +4
Lines 9191 9482 +291
Branches 3122 3208 +86
==========================================
+ Hits 7463 7506 +43
- Misses 1728 1976 +248
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
568944f
to
73941ba
Compare
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.
кек, сам CarouselBase
я ведь не посмотрел x)) из-за того, что в интерфейсе он по-умолчанию был скрыт 🌚
пока вот такие комменты, ещё гляну собственно CarouselBase
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/BaseGallery/CarouselBase/CarouselBase.tsx
Outdated
Show resolved
Hide resolved
packages/vkui/src/components/BaseGallery/CarouselBase/CarouselBase.tsx
Outdated
Show resolved
Hide resolved
packages/vkui/src/components/BaseGallery/CarouselBase/CarouselBase.tsx
Outdated
Show resolved
Hide resolved
packages/vkui/src/components/BaseGallery/CarouselBase/CarouselBase.tsx
Outdated
Show resolved
Hide resolved
packages/vkui/src/components/BaseGallery/CarouselBase/CarouselBase.tsx
Outdated
Show resolved
Hide resolved
4a9a581
to
2410865
Compare
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.
Согласен, грандиозная работа. 💯
Блокирующий только вопрос по функции getShiftedIndexes
, остальное опционно.
packages/vkui/src/components/BaseGallery/CarouselBase/CarouselBase.tsx
Outdated
Show resolved
Hide resolved
packages/vkui/src/components/BaseGallery/CarouselBase/CarouselBase.tsx
Outdated
Show resolved
Hide resolved
packages/vkui/src/components/BaseGallery/CarouselBase/helpers.ts
Outdated
Show resolved
Hide resolved
70e9fa1
to
8306a12
Compare
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.
🥇 Спасибо!
* feat(Carousel): add Gallery with loop
Создала отдельным компонентом, потому что пришлось переделать механизм "пролистывания" слайда, чтобы не заафектить уже работающую
BaseGallery
К
v6
переедем на общий механизм