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: qrcode canvas supports style configuration #48194
Conversation
Run & review this pull request in StackBlitz Codeflow. |
👁 Visual Regression Report for PR #48194 Passed ✅
🎊 Congrats! No visual-regression diff found. |
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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature #48194 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 745 745
Lines 12987 12989 +2
Branches 3406 3408 +2
=========================================
+ Hits 12987 12989 +2 ☔ View full report in Codecov by Sentry. |
原始问题是啥? |
原始问题是:我们有时候需要配置 width:'100%', 但是现在只能用size来配置固定px。 |
给个重现? |
https://codepen.io/thinkasany/pen/JjVOYyq?editors=0010 外部div witdth 200px了,但是内部svg还是会保留默认的size 现在的代码用了‘100%’ 会出现下面这种情况,classname为 |
这样不行的,QRCode 的默认大小就是 160,如果没有覆盖 size,大小不应该变化。 另外 style.width + style.height 设置的实际上是 QRCode 外框的大小,而非二维码本身的大小,这两个存在分开配置的可能性。 如果需要二维码大小跟踪外框走,建议 |
@afc163 |
还有就是 size 目前有约束 只能是number类型,当时试过pr |
我感觉可以加一个 |
现在就是只有修改 |
可以在 antd 这层放开类型限制。 |
我曾经尝试过 #48019 ,ci 过不去 https://github.com/ant-design/ant-design/actions/runs/8386601484/job/22967339294 |
as 一下? |
顺便加个 max-width: 100% 的属性 |
'> canvas': {
alignSelf: 'stretch',
flex: 'auto',
minWidth: 0,
minHeight: 0,
maxWidth: '100%',
maxHeight: '100%',
}, |
@afc163 老板辛苦你再看一眼 🙏, 希望没有耽误你太多时间精力,如果还是不能解决的话,我再想想其他方式去解决这个问题好了,我又重新整理了一下组织语言,最终汇集成以下两点:
|
bugfix or feature? |
感觉也可以算是feature,因为原来的预期是只用配置border宽度就够了(二维码用size控制),现在多了一个style支持调整qrcode内部二维码的宽高 |
max-width: 100% 不能解决原始问题么? |
@afc163 ,辛苦老板再帮忙看看,上面提到的都调整好了。 |
Co-authored-by: lijianan <574980606@qq.com> Signed-off-by: thinkasany <480968828@qq.com>
需要加个 bordered 属性。 |
expect(container.querySelector<HTMLDivElement>('.ant-qrcode')).toHaveStyle( | ||
'width: 100px; height: 80px', | ||
it('correct style order for canvas', () => { | ||
const { container } = render(<QRCode value="test" size={80} style={{ width: '100%' }} />); |
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.
加了新特性的话,需要新增 test case,不要动原来用例
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.
[中文版模板 / Chinese template]
🤔 This is a ...
🔗 Related issue link
关联修改 #45815 #48053
💡 Background and solution
qrcode修改需要支持style的配置才能实现‘100%’,之前的pr只修复了外层的div的宽度,实际上我们需要让canvas也生效,用style的宽度来支持修改是react.qrcode 作者推荐的写法,他不建议使用size来做‘100%’,所以需要佬们看一下,怎么支持一下。
目前我的修改代码实际上只有在配置了witdh才会覆盖,如果没配置witdth应该值还是原来的,不会影响到原来的代码逻辑。
📝 Changelog
☑️ Self-Check before Merge