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: add tabindex option for paragraph button #48567

Merged
merged 12 commits into from Apr 24, 2024

Conversation

nova1751
Copy link
Contributor

@nova1751 nova1751 commented Apr 21, 2024

中文版模板 / Chinese template

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

The Paragraph component didn't support the tabIndex option for edit and copy button. I achieved this feature.

📝 Changelog

Language Changelog
🇺🇸 English Add tabIndex option for button in Paragraph component
🇨🇳 Chinese Paragraph 组件的按钮添加 tabIndex 选项

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

Copy link

stackblitz bot commented Apr 21, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Contributor

github-actions bot commented Apr 21, 2024

👁 Visual Regression Report for PR #48567 Passed ✅

🎯 Target branch: feature (5c2e6e3)
📖 View Full Report ↗︎

🎊 Congrats! No visual-regression diff found.

Copy link
Contributor

github-actions bot commented Apr 21, 2024

Preview is ready

Copy link

codesandbox-ci bot commented Apr 21, 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

codecov bot commented Apr 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (5c2e6e3) to head (e0c0004).

Additional details and impacted files
@@            Coverage Diff            @@
##           feature    #48567   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          745       745           
  Lines        12979     12982    +3     
  Branches      3403      3404    +1     
=========================================
+ Hits         12979     12982    +3     

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

@afc163
Copy link
Member

afc163 commented Apr 23, 2024

看下覆盖率。

@nova1751
Copy link
Contributor Author

看下覆盖率。

我不是很懂为何会报错,是需要为 locale 添加一个 test case吗?

@afc163
Copy link
Member

afc163 commented Apr 23, 2024

看上去是的。

@nova1751
Copy link
Contributor Author

看上去是的。

locale加上test case好像没问题了,不过为啥之前locale没报这个错误呢😂

@li-jia-nan
Copy link
Member

新功能需要提交到 feature 分支,另外 api 文档里面要补一下版本号

@nova1751 nova1751 changed the base branch from master to feature April 24, 2024 01:59
@nova1751
Copy link
Contributor Author

新功能需要提交到 feature 分支,另外 api 文档里面要补一下版本号

版本号的话使用当前版本号的下一个版本号比较好吗

@li-jia-nan
Copy link
Member

版本号的话使用当前版本号的下一个版本号比较好吗

嗯,写 5.17.0

@li-jia-nan
Copy link
Member

另外分支需要清理一下,你把 master 分支的提交带过来了,PR 里面不应该出现不相关的 commit

@nova1751
Copy link
Contributor Author

另外分支需要清理一下,你把 master 分支的提交带过来了,PR 里面不应该出现不相关的 commit

OK, 我 cherry-pick 一下。

@nova1751
Copy link
Contributor Author

奇怪,为什么codecov又出问题了

@nova1751
Copy link
Contributor Author

还是不懂为何覆盖率继续报错了,能提供些建议吗?@afc163 @li-jia-nan

@li-jia-nan
Copy link
Member

点进去详情看一下,有一行没覆盖到:

image

@yoyo837
Copy link
Contributor

yoyo837 commented Apr 24, 2024

image

点进去可以看到

@nova1751
Copy link
Contributor Author

nova1751 commented Apr 24, 2024

点进去详情看一下,有一行没覆盖到:

image

但我添加了一个test case了,还是报错,这个由于是paragraph内部的组件我也不太好测试,之前加上去覆盖率通过了,但不知道为啥切换下分支覆盖率又没通过了,不是很懂接下来该怎么做?🤔

@yoyo837
Copy link
Contributor

yoyo837 commented Apr 24, 2024

要覆盖这个,应该需要没有传入locale,让他走默认值的情况

@nova1751
Copy link
Contributor Author

nova1751 commented Apr 24, 2024

要覆盖这个,应该需要没有传入locale,让他走默认值的情况

我之前添加了这个test case覆盖率通过了,但不知道为什么切换分支后又不通过了,这个例子应该就是有走默认值的情况
image

@yoyo837
Copy link
Contributor

yoyo837 commented Apr 24, 2024

要覆盖这个,应该需要没有传入locale,让他走默认值的情况

我之前添加了这个test case覆盖率通过了,但不知道为什么切换分支后又不通过了,这个例子应该就是没有走默认值的情况 image

同步一下上游看看?

但是这个用例就是需要走默认值才能覆盖到。

@yoyo837
Copy link
Contributor

yoyo837 commented Apr 24, 2024

新增一个走默认值的即可。

@nova1751
Copy link
Contributor Author

新增一个走默认值的即可。

抱歉我刚刚的表述有点问题,我的意思是我新添加了一个走默认值的测试用例,就是上面那张截图,当初向master分支合的时候覆盖率通过了,但不知道为啥向feature分支合的时候又报了这个错误,接下来我就不懂该怎么操作了,由于这个locale是内部用的hooks传的,我也不放便单独拿出来测试,所以有点疑问。

@li-jia-nan
Copy link
Member

再新增一个非默认值的用例试试?

@li-jia-nan
Copy link
Member

可以先在本地生成测试覆盖率报告看一下:npm run test xxxx/xxxx -- --coverage

nova1751 and others added 6 commits April 24, 2024 16:40
Signed-off-by: lijianan <574980606@qq.com>
Signed-off-by: lijianan <574980606@qq.com>
Signed-off-by: lijianan <574980606@qq.com>
Signed-off-by: lijianan <574980606@qq.com>
Signed-off-by: lijianan <574980606@qq.com>
@li-jia-nan li-jia-nan merged commit 9acde83 into ant-design:feature Apr 24, 2024
63 checks passed
@nova1751 nova1751 deleted the feat/typography branch April 25, 2024 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants