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

fix(style): keep checkbox.group style the same as v4 #42103

Merged
merged 7 commits into from
May 9, 2023

Conversation

BoyYangzai
Copy link
Contributor

@BoyYangzai BoyYangzai commented May 1, 2023

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

📝 Changelog

Language Changelog
🇺🇸 English fix(style): keep checkbox.group style the same as v4
🇨🇳 Chinese checkbox.group style 风格与v4保持一致

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

🚀 Summary

🤖 Generated by Copilot at 1afca5d

Improved the styling of checkbox and checkbox.Group components to support flex and horizontal layouts. Modified components/checkbox/style/index.ts to adjust the display and margin properties.

🔍 Walkthrough

🤖 Generated by Copilot at 1afca5d

  • Fix layout issue of checkbox inside flex container by changing display to inline-block and adding margin-right to checkbox group items (link)
  • Remove margin-inline-start from checkbox wrapper to avoid unnecessary margins when checkbox group is aligned horizontally (link)

@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2023

[`${checkboxCls}-group-item`]: {
...resetComponent(token),

marginRight: '8px',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
marginRight: '8px',
marginInlineEnd: token.marginXS

@codecov
Copy link

codecov bot commented May 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (711c9d6) 100.00% compared to head (f432511) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #42103   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          617       617           
  Lines        10545     10544    -1     
  Branches      2883      2883           
=========================================
- Hits         10545     10544    -1     
Impacted Files Coverage Δ
components/checkbox/style/index.ts 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

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

@@ -36,6 +36,13 @@ export const genCheckboxStyle: GenerateStyle<CheckboxToken> = (token) => {
...resetComponent(token),

display: 'inline-flex',
flexWrap: 'wrap',
columnGap: '8px',
Copy link
Member

Choose a reason for hiding this comment

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

用 token 哈

@MadCcc
Copy link
Member

MadCcc commented May 8, 2023

image
这分的太开了吧?

columnGap: '8px',

// Group > Grid
'.ant-row': {
Copy link
Member

Choose a reason for hiding this comment

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

v4 没有这个样式吧

Comment on lines 43 to 45
'.ant-row': {
width: '100%',
},
Copy link
Member

@MadCcc MadCcc May 8, 2023

Choose a reason for hiding this comment

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

Suggested change
'.ant-row': {
width: '100%',
},
[`> ${token.antCls}-row`]: {
flex: 1,
},
  1. ant 前缀要用 token,这是动态的
  2. flex 布局下用 flex: 1 撑满
  3. > 只选择子代

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@MadCcc
Copy link
Member

MadCcc commented May 8, 2023

image
这个 diff 是没有用 token 导致 compact 主题下间距不对,改了就好了。
这个 PR 应该只影响 grid 布局的 snapshot

@BoyYangzai
Copy link
Contributor Author

image 这个 diff 是没有用 token 导致 compact 主题下间距不对,改了就好了。 这个 PR 应该只影响 grid 布局的 snapshot

get

@MadCcc MadCcc merged commit 52d91ce into ant-design:master May 9, 2023
52 checks passed
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.

Checkbox.Group 排版样式与 Antd v4 保持一致
3 participants