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

[WIP] fix: 修复明细表底部存在空白行的问题 (close #2958) #2967

Open
wants to merge 19 commits into
base: next
Choose a base branch
from

Conversation

linwrui
Copy link
Contributor

@linwrui linwrui commented Nov 8, 2024

👀 PR includes

✨ Feature

  • New feature

🎨 Enhance

  • Code style optimization
  • Refactoring
  • Change the UI
  • Improve the performance
  • Type optimization

🐛 Bugfix

🔧 Chore

  • Test case
  • Docs / demos update
  • CI / workflow
  • Release version
  • Other ()

📝 Description

我看了一下 v1 版本的滚动条是在面板之外的,所以这里应该是旧代码做了空间的预留,但是 v2的滚动条已经调整成悬浮在 panel-bbox 内,所以这里也就没必要再预留滚动条的高度了

🖼️ Screenshot

Before After
企业微信截图_17310463696331 企业微信截图_17310462938748

🔗 Related issue link

closes #2958 #2295

🔍 Self-Check before the merge

  • Add or update relevant docs.
  • Add or update relevant demos.
  • Add or update test case.
  • Add or update relevant TypeScript definitions.

Copy link

vercel bot commented Nov 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
antvis-s2 ❌ Failed (Inspect) Nov 23, 2024 9:53am

@linwrui linwrui changed the title fix: 修复明细表底部存在空白行的问题 (#2958) fix: 修复明细表底部存在空白行的问题 (close #2958) Nov 8, 2024
@linwrui
Copy link
Contributor Author

linwrui commented Nov 8, 2024

@lijinke666 为啥本地没法执行测试脚本?参考贡献指南操作的

image

@lijinke666
Copy link
Member

lijinke666 commented Nov 8, 2024

什么系统? 看起来是 Windows? Mac 未复现

image

试试直接运行 pnpm --filter @antv/s2 test -- -u 或者 pnpm --filter @antv/s2 test "--" "-u"

@linwrui
Copy link
Contributor Author

linwrui commented Nov 8, 2024

什么系统? 看起来是 Windows? Mac 未复现

image

试试直接运行 pnpm --filter @antv/s2 test -- -u 或者 pnpm --filter @antv/s2 test "--" "-u"

是 windows ,后面那个命令可以执行

@lijinke666 lijinke666 linked an issue Nov 8, 2024 that may be closed by this pull request
5 tasks
@lijinke666
Copy link
Member

#2295

@lijinke666 lijinke666 linked an issue Nov 11, 2024 that may be closed by this pull request
5 tasks
@linwrui linwrui changed the title fix: 修复明细表底部存在空白行的问题 (close #2958) [WIP] fix: 修复明细表底部存在空白行的问题 (close #2958) Nov 12, 2024
@github-actions github-actions bot removed the pr(fix) bug fix label Nov 12, 2024
@lijinke666
Copy link
Member

@linwrui 看你改为了 WIP, 是遇到啥问题了吗, 近期会发正式版, 需要跟车的话还请尽快处理下 PR

@linwrui
Copy link
Contributor Author

linwrui commented Nov 16, 2024

@linwrui 看你改为了 WIP, 是遇到啥问题了吗, 近期会发正式版, 需要跟车的话还请尽快处理下 PR

这个调整对布局和滚动条相关的单测影响较大,需要再看看怎么调整

# Conflicts:
#	packages/s2-core/__tests__/spreadsheet/__snapshots__/multi-line-text-spec.ts.snap
#	packages/s2-react/__tests__/unit/components/advanced-sort/__snapshots__/index-spec.tsx.snap
#	packages/s2-react/__tests__/unit/components/drill-down/__snapshots__/index-spec.tsx.snap
#	packages/s2-react/__tests__/unit/components/export/__snapshots__/index-spec.tsx.snap
#	packages/s2-react/__tests__/unit/components/header/__snapshots__/index-spec.tsx.snap
#	packages/s2-react/__tests__/unit/components/pagination/__snapshots__/index-spec.tsx.snap
#	packages/s2-react/__tests__/unit/components/sheets/__snapshots__/index-spec.tsx.snap
#	packages/s2-react/__tests__/unit/components/switcher/__snapshots__/index-spec.tsx.snap
#	packages/s2-react/__tests__/unit/components/tooltip/__snapshots__/index-spec.tsx.snap
@linwrui
Copy link
Contributor Author

linwrui commented Nov 23, 2024

@linwrui 看你改为了 WIP, 是遇到啥问题了吗, 近期会发正式版, 需要跟车的话还请尽快处理下 PR

不知道是不是 windows 系统的问题,我直接切换到 next 支线执行 pnpm core:test -- -u 也是不通过的,有13个用例是 FAIL,而且更新的快照结果也和线上的不一致

This reverts commit 784f858.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next 2.0-next 版本的问题
Projects
None yet
3 participants