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

fix: card extra add padding #39646

Merged
merged 6 commits into from Dec 21, 2022
Merged

fix: card extra add padding #39646

merged 6 commits into from Dec 21, 2022

Conversation

JarvisArt
Copy link
Contributor

[中文版模板 / 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

close #39643

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English card extra add padding，v5 is missing
🇨🇳 Chinese card extra 添加 padding，v5漏加了

☑️ 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

@github-actions
Copy link
Contributor

github-actions bot commented Dec 19, 2022 •
edited

@MadCcc
Copy link
Member

MadCcc commented Dec 19, 2022

#39643 (comment)
看这个应该是把 cardHeadHeight 干掉吧，有 min-height 就行了

@afc163
Copy link
Member

afc163 commented Dec 19, 2022

看看和这个是否结论一致 #15668

@JarvisArt
Copy link
Contributor Author

我对比了下v4跟v5，v4有padding，v5的padding为0，但是v5中的size="small"却有padding，所以还是缺少这个padding

@MadCcc
Copy link
Member

MadCcc commented Dec 19, 2022

给个 debug demo 测一下

@JarvisArt
Copy link
Contributor Author

image

@JarvisArt
Copy link
Contributor Author

给个 debug demo 测一下

文档的第一个demo就可以看出了
image

@codecov
Copy link

codecov bot commented Dec 19, 2022 •
edited

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (b4e9750) compared to base (527c71d).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@            Coverage Diff            @@
##            master    #39646   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          556       556           
  Lines         9607      9607           
  Branches      2718      2718           
=========================================
  Hits          9607      9607

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@MadCcc
Copy link
Member

MadCcc commented Dec 19, 2022

正好有个 demo 的 image test 覆盖了
image

@MadCcc
Copy link
Member

MadCcc commented Dec 19, 2022

cardHeadHeight 的问题也要管一下，看看这个应该可以用 cardHeaderHeight 替代了

@JarvisArt
Copy link
Contributor Author

cardHeadHeight 的问题也要管一下，看看这个应该可以用 cardHeaderHeight 替代了

cardHeadHeight 跟 cardHeadPadding 不冲突，

minHeight: cardHeadHeight，如果title的元素高度超过minHeight，那也是要撑开的，至于cardHeadHeight为undefined要看一下

@MadCcc
Copy link
Member

MadCcc commented Dec 19, 2022

至于cardHeadHeight为undefined要看一下

没有初始化，应该是之前改动的 bug，可以改了

@JarvisArt
Copy link
Contributor Author

嗯嗯，应该是后面的改动把 cardHeaderHeight 跟 cardHeadHeight 搞混了。

@JarvisArt
Copy link
Contributor Author

@MadCcc v4中的minHeight是48px，v5的要跟他一样是吧， 我看cardHeaderHeight现在计算出来是56px

@MadCcc
Copy link
Member

MadCcc commented Dec 19, 2022

这个 min-height 应该是为了 extra 设置的，防止高度缩了。可以试下 extra 换成标准 Button 高度会不会撑开，应该是保持一样的

@JarvisArt
Copy link
Contributor Author

v4:
image

v5(现在表现一致了):
image

@MadCcc
Copy link
Member

MadCcc commented Dec 19, 2022

#15668 按这个说法应该还是 56 才对？

@JarvisArt
Copy link
Contributor Author

#15668 按这个说法应该还是 56 才对？

这里面的这个说法是不是在v4没有做相应的修改啊，因为按里面说的做是需要把padding去掉的，然后子元素垂直居中。

@MadCcc
Copy link
Member

MadCcc commented Dec 20, 2022

覆盖率掉了，merge 一下 base 吧

@MadCcc
Copy link
Member

MadCcc commented Dec 20, 2022

image
看了下 v4 这里是居中的，但是 v5 不是，也需要改改

@JarvisArt
Copy link
Contributor Author

v4的也不是居中，这里做了特殊处理，是size="small"才会垂直居中
image

@MadCcc
Copy link
Member

MadCcc commented Dec 20, 2022

右边的 More 和 title 是居中对齐的，v5 没有
image

@JarvisArt
Copy link
Contributor Author

右边的 More 和 title 是居中对齐的，v5 没有 !

done

@MadCcc
Copy link
Member

MadCcc commented Dec 21, 2022

merge 一下 base 看看 argos

@JarvisArt
Copy link
Contributor Author

咋没见到 argos

@MadCcc
Copy link
Member

MadCcc commented Dec 21, 2022

改了之后 header 变矮了？

@afc163
Copy link
Member

afc163 commented Dec 21, 2022

图片

@JarvisArt
Copy link
Contributor Author

改了之后 header 变矮了？

原先的 header 高度是58px，改完后56px
image
image

@MadCcc
Copy link
Member

MadCcc commented Dec 21, 2022

感觉可以接受？56 是 4 的倍数，更合理些

@JarvisArt
Copy link
Contributor Author

56也是刚好可以被计算出来，符合里面token变量的标准
image

@MadCcc MadCcc merged commit 78592a5 into ant-design:master Dec 21, 2022
@vagusX vagusX mentioned this pull request Dec 26, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MadCcc MadCcc MadCcc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Card 组件在仅传入 extra 时样式异常
3 participants
@JarvisArt @MadCcc @afc163