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.

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(HorizontalScroll, CardScroll): Add testid props for arrows #8032

Merged

Conversation

andrey-medvedev-vk
Copy link
Contributor

@andrey-medvedev-vk andrey-medvedev-vk commented Dec 3, 2024


  • Unit-тесты
  • Документация фичи
  • Release notes

Описание

Добавляем свойства для прокидывания data-testid на стрелки.
Убираем старые, зашитые в код testId.

Release notes

Улучшения

  • HorizontalScroll: добавлена возможность передать data-testid в стрелки навигации с помощью новых свойств startArrowTestId и endArrowTestId.
  • CardScroll: добавлена возможность передать data-testid в стрелки навигации с помощью новых свойств startArrowTestId и endArrowTestId.

Remove test specific test ids to prefer props
Copy link
Contributor

github-actions bot commented Dec 3, 2024

size-limit report 📦

Path Size
JS 386.99 KB (+0.05% 🔺)
JS (gzip) 117.08 KB (+0.05% 🔺)
JS (brotli) 96.48 KB (-0.13% 🔽)
JS import Div (tree shaking) 1.56 KB (0%)
CSS 337.59 KB (0%)
CSS (gzip) 42.83 KB (0%)
CSS (brotli) 33.86 KB (0%)

Copy link

codesandbox-ci bot commented Dec 3, 2024

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.

Copy link
Contributor

github-actions bot commented Dec 3, 2024

e2e tests

⚠️ Some screenshots were failed. See Playwright Report.

Playwright Report

Copy link
Contributor

github-actions bot commented Dec 3, 2024

👀 Docs deployed

Commit 6ab0a1b

Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.48%. Comparing base (0eeffef) to head (6ab0a1b).
Report is 31 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8032   +/-   ##
=======================================
  Coverage   95.47%   95.48%           
=======================================
  Files         393      392    -1     
  Lines       11188    11153   -35     
  Branches     3708     3677   -31     
=======================================
- Hits        10682    10649   -33     
+ Misses        506      504    -2     
Flag Coverage Δ
unittests 95.48% <ø> (+<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.

@andrey-medvedev-vk andrey-medvedev-vk changed the title feat(HorizontalScroll): Add testid props for arrows feat(HorizontalScroll, CardScroll): Add testid props for arrows Dec 3, 2024
@andrey-medvedev-vk andrey-medvedev-vk marked this pull request as ready for review December 3, 2024 07:55
@andrey-medvedev-vk andrey-medvedev-vk requested a review from a team as a code owner December 3, 2024 07:55
Copy link
Contributor

@EldarMuhamethanov EldarMuhamethanov left a comment

Choose a reason for hiding this comment

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

👍

@andrey-medvedev-vk
Copy link
Contributor Author

Обновил JSDoc в соответствии с новым правилом для пропов testid6ab0a1b

@andrey-medvedev-vk andrey-medvedev-vk merged commit ff97040 into master Dec 10, 2024
28 checks passed
@andrey-medvedev-vk andrey-medvedev-vk deleted the mendrew/add-test-id-to-horizontal-scroll-arrows branch December 10, 2024 14:03
@inomdzhon
Copy link
Contributor

inomdzhon commented Dec 10, 2024

сорри, немного запоздал с ревью

в случае HorizontalScroll и CardScroll по семантике не стоило ли назвать prevArrowTestId и nextArrowTestId?

UPD
или prevButtonTestId и nextButtonTestId

@andrey-medvedev-vk
Copy link
Contributor Author

сорри, немного запоздал с ревью

в случае HorizontalScroll и CardScroll по семантике не стоило ли назвать prevArrowTestId и nextArrowTestId?

UPD или prevButtonTestId и nextButtonTestId

@inomdzhon ты чертовски прав, я как-то не заметил и больше думал про имена из логических свойств.
Сделал PR #8062

@vkcom-publisher
Copy link
Contributor

v7.1.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants