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: There are style issues with Table and other elements under Table expand #47190

Closed
wants to merge 4 commits into from

Conversation

crazyair
Copy link
Member

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

上次加的 pr #23602
这次修复的 issue #47189

📝 Changelog

Language Changelog
🇺🇸 English There are style issues with Table and other elements under Table expand
🇨🇳 Chinese Table expand 下有 Table 和其他元素的样式问题

☑️ 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 Jan 27, 2024

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

Copy link
Contributor

github-actions bot commented Jan 27, 2024

Visual Regression Report for PR #47190 Passed ✅

Target branch: master (52ad8b0)


Congrats! No visual-regression diff found

Copy link
Contributor

github-actions bot commented Jan 27, 2024

Preview Is ready

Copy link
Contributor

github-actions bot commented Jan 27, 2024

size-limit report 📦

Path Size
./dist/antd.min.js 331.34 KB (-56 B 🔽)
./dist/antd-with-locales.min.js 377.1 KB (-131 B 🔽)

Copy link

codesandbox-ci bot commented Jan 27, 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.

Latest deployment of this branch, based on commit 83c6e79:

Sandbox Source
antd reproduction template (forked) Configuration

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3365a84) 100.00% compared to head (83c6e79) 100.00%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #47190   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          732       732           
  Lines        12563     12563           
  Branches      3295      3295           
=========================================
  Hits         12563     12563           

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

@@ -38,7 +38,7 @@ const genSizeStyle: GenerateStyle<TableToken, CSSObject> = (token) => {

[`${componentCls}-tbody`]: {
// ========================= Nest Table ===========================
[`${componentCls}-wrapper:only-child ${componentCls}`]: {
[`${componentCls}-wrapper ${componentCls}`]: {
Copy link
Member

Choose a reason for hiding this comment

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

这个 :only-child 应该是有用的,blame 一下。

Copy link
Member Author

Choose a reason for hiding this comment

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

描述早写清楚了

@crazyair
Copy link
Member Author

上次的 pr
第一个 issue 没问题
image

image

第二个 issue 看了也没问题

@afc163
Copy link
Member

afc163 commented Jan 29, 2024

目前这个样式容易误伤不是直接内嵌的情况(比如放一个 Card 里面再嵌套 Table),建议用 > 来精确指定对应的子 Table。https://stackblitz.com/edit/react-rvtz1f-tgwzvl?file=demo.tsx

@crazyair
Copy link
Member Author

crazyair commented Jan 29, 2024

目前这个样式容易误伤不是直接内嵌的情况(比如放一个 Card 里面再嵌套 Table),建议用 > 来精确指定对应的子 Table。https://stackblitz.com/edit/react-rvtz1f-tgwzvl?file=demo.tsx

image
不能用 > 这里还有2级div

目前这个样式容易误伤不是直接内嵌的情况(比如放一个 Card 里面再嵌套 Table),建议用 > 来精确指定对应的子 Table。https://stackblitz.com/edit/react-rvtz1f-tgwzvl?file=demo.tsx

你这个问题还没理解啊?这个 demo 没有写 {open && <div>111</div>},所以复现不出来,需要在 Table 同级写个 div 来复现
image

@afc163
Copy link
Member

afc163 commented Jan 29, 2024

原来的写法用 only-child 限制了一部分误伤,但依然有误伤问题。如果去掉,误伤问题将被放大。

因此需要另外找一种限制场景的样式定制方式,避免 expand render 内其他无需右移的 Table 被影响到。

@crazyair
Copy link
Member Author

原来的写法用 only-child 限制了一部分误伤,但依然有误伤问题。如果去掉,误伤问题将被放大。

因此需要另外找一种限制场景的样式定制方式,避免 expand render 内其他无需右移的 Table 被影响到。

之前只是对 ant-table 进行了 margin 现在依然是对 ant-table 进行 margin 没有改变,只是之前 ant-table-wrapper 必须是 only-child,现在不管 <Table /> 是不是一个,最后也都只是给 ant-table 元素 margin

@afc163
Copy link
Member

afc163 commented Jan 29, 2024

必须是 only-child 就是希望避免一部分场景的误伤,意思是如果里面不是直接放一个 Table,那很有可能用户是有自己的样式需要,不需要做默认边距调整。当然 only-child 并不准。

@crazyair
Copy link
Member Author

必须是 only-child 就是希望避免一部分场景的误伤,意思是如果里面不是直接放一个 Table,那很有可能用户是有自己的样式需要,不需要做默认边距调整。当然 only-child 并不准。

最终是给 ant-table-wrapper 下面的 ant-tablemargin,那下面如果有多个 Table 是不是都需要加 margin https://stackblitz.com/edit/react-rvtz1f-chjvps?file=demo.tsx

@afc163
Copy link
Member

afc163 commented Jan 29, 2024

这里情况很多,可能只能加属性来解决。

@afc163
Copy link
Member

afc163 commented Jan 29, 2024

我是希望一并将这个问题解决掉:https://stackblitz.com/edit/react-rvtz1f-tgwzvl?file=demo.tsx

@crazyair
Copy link
Member Author

crazyair commented Jan 29, 2024

我是希望一并将这个问题解决掉:https://stackblitz.com/edit/react-rvtz1f-tgwzvl?file=demo.tsx

这个例子,我其实认为是合理的,因为当时就是只对 ant-tablemargin,那也算 bug as feature 了。。

首先这个是问题吗,比如 expandedRowRender 就返回个 div 是否需要加 margin,如果能这样改是最好的

@afc163
Copy link
Member

afc163 commented Jan 29, 2024

           expandedRowRender: () => (
              <>
                <Table.ExpandColumnPlacehoder /> // 这个用来解决左侧内容对齐问题。
                <Table columns={columns} dataSource={data} />
              </>
            ),

先抛一个想法。

@crazyair crazyair marked this pull request as draft January 30, 2024 08:13
@afc163 afc163 closed this Feb 2, 2024
@afc163 afc163 deleted the fix-47189 branch February 8, 2024 05:14
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

2 participants