Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(Gallery): add handle click to bullet #7213

Closed
wants to merge 1 commit into from
Closed

feat(Gallery): add handle click to bullet #7213

wants to merge 1 commit into from

Conversation

EldarMuhamethanov
Copy link
Contributor

Описание

Необходимо добавить обработку кликов на bullets

Изменения

  • Вынес компонент GalleryBullets, чтобы избавиться от дублирования
  • Добавил обработку кликов на bullet с переходам к нужному слайду
  • Добавил пропс bulletsClassName класснейм для контейнера с bullets
  • Добавил пропс slideClassName классней для слайда в Gallery

@EldarMuhamethanov EldarMuhamethanov requested a review from a team as a code owner July 19, 2024 08:04
@github-actions GitHub Actions
Copy link
Contributor

size-limit report 📦

Path Size
JS 373.62 KB (+0.04% 🔺)
JS (gzip) 114.45 KB (+0.07% 🔺)
JS (brotli) 94.3 KB (+0.12% 🔺)
JS import Div (tree shaking) 1.42 KB (0%)
CSS 301.45 KB (+0.02% 🔺)
CSS (gzip) 38.43 KB (+0.03% 🔺)
CSS (brotli) 30.92 KB (+0.04% 🔺)

@codesandbox-ci CodeSandbox CI
Copy link

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.

@github-actions GitHub Actions
Copy link
Contributor

e2e tests

Playwright Report

@github-actions GitHub Actions
Copy link
Contributor

👀 Docs deployed

Commit 29aa64b

@codecov Codecov
Copy link

codecov bot commented Jul 19, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.07%. Comparing base (65651ad) to head (29aa64b).
Report is 236 commits behind head on master.

Files with missing lines Patch % Lines
...ents/BaseGallery/GalleryBullets/GalleryBullets.tsx 55.55% 4 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #7213   +/-   ##
=======================================
  Coverage   84.06%   84.07%           
=======================================
  Files         361      362    +1     
  Lines       10934    10939    +5     
  Branches     3598     3598           
=======================================
+ Hits         9192     9197    +5     
  Misses       1742     1742
Flag Coverage Δ
unittests 84.07% <63.63%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mendrew
mendrew approved these changes Jul 19, 2024
View reviewed changes
Copy link
Contributor

@mendrew mendrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здорово.

Немного только смущает размер области по которой можно кликнуть/нажать у Bullets. Как-будто она маловата. И по хорошему её бы тогда увеличить.

packages/vkui/src/components/BaseGallery/BaseGallery.tsx
bullets={bullets}
slideIndex={slideIndex}
onChange={onChange}
bulletsClassName={bulletsClassName}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Этот проп я бы просто назвал className у самого компонента GalleryBullets.

mendrew
mendrew reviewed Jul 19, 2024
View reviewed changes
Copy link
Contributor

@mendrew mendrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А ещё надо бы подумать про доступность.

Bullets перестают быть декоративным элементом и становятся интерактивным.
Сейчас это просто получаются кнопки без названия. 🤔

@inomdzhon
Copy link
Contributor

Здорово.

Немного только смущает размер области по которой можно кликнуть/нажать у Bullets. Как-будто она маловата. И по хорошему её бы тогда увеличить.

По поводу размеров надо будет пойти к @VKCOM/vkui-design

@vkcom-publisher vkcom-publisher added pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности and removed pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности labels Jul 26, 2024
@vkcom-publisher vkcom-publisher added pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности and removed pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности labels Aug 4, 2024
@vkcom-publisher vkcom-publisher added pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности and removed pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности labels Aug 12, 2024
@vkcom-publisher vkcom-publisher added pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности and removed pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности labels Aug 20, 2024
@EldarMuhamethanov EldarMuhamethanov marked this pull request as draft August 26, 2024 08:34
@vkcom-publisher vkcom-publisher added the pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности label Sep 2, 2024
@vkcom-publisher
Copy link
Contributor

PR закрыт из-за отсутствия активности в течение последних 14 дней. Если это произошло по ошибке или изменения все ещё актуальны, откройте PR повторно.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mendrew mendrew mendrew approved these changes

Assignees
No one assigned
Labels
pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности
Projects
None yet
Milestone
No milestone
4 participants
@EldarMuhamethanov @inomdzhon @vkcom-publisher @mendrew