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

feat(form): improve scrollToField logic #48211

Merged

Conversation

Wxh16144
Copy link
Member

@Wxh16144 Wxh16144 commented Apr 1, 2024

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

改进表单 scrollToField 方法逻辑 ,优先从 item Ref 中获取元素,等待 #48210 合并后可以 close #28869

解决方案讨论 ref:#45410 (comment)

📝 Changelog

Language Changelog
🇺🇸 English Fix Form scrollToFirstError cannot work on Upload.
🇨🇳 Chinese 修复 Form 下出现错误时无法滚动到 Upload 组件的问题。

☑️ 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 Apr 1, 2024

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

Copy link
Contributor

github-actions bot commented Apr 1, 2024

👁 Visual Regression Report for PR #48211 Passed ✅

🎯 Target branch: feature (8026366)
📖 View Full Report ↗︎

🎊 Congrats! No visual-regression diff found.

Copy link
Contributor

github-actions bot commented Apr 1, 2024

Preview is ready

Copy link

codesandbox-ci bot commented Apr 1, 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.

Copy link

codecov bot commented Apr 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (8026366) to head (911c997).
Report is 1 commits behind head on feature.

Additional details and impacted files
@@            Coverage Diff            @@
##           feature    #48211   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          744       744           
  Lines        12896     12903    +7     
  Branches      3382      3384    +2     
=========================================
+ Hits         12896     12903    +7     

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

@Wxh16144 Wxh16144 force-pushed the wuxh/feat-form-scrollToField-ref branch from fa816cd to e244b93 Compare April 1, 2024 17:10
@Wxh16144
Copy link
Member Author

Wxh16144 commented Apr 2, 2024

get 一个新知识,分支覆盖率和行覆盖率

相同逻辑代码

写法1:

function fn(a, b) {
  if (a > b) {
    return 'a>b'
  } else {
    if (a === b) {
      return 'a=b'
    }
  }
  return "???"
}

export default fn

写法2:

function fn(a, b) {
  if (a > b) {
    return 'a>b'
  } else {
    if (a === b) return 'a=b'
  }
  return "???"
}

export default fn

走同一份测试用例

import { expect, describe, it } from 'vitest'
import fn from './sum'


describe('sum', () => {
  it('a>b', () => {
    expect(fn(2, 1)).toBe('a>b')
  })

  it('other', () => {
    expect(fn(1, 2)).toBe('???')
  })
  
  it('case a=b', () => {
    expect(fn(1, '1')).toBe('???')
  })
})

最终的覆盖率报告有所不同

image

@Wxh16144
Copy link
Member Author

Wxh16144 commented Apr 2, 2024

Co-authored-by: lijianan <574980606@qq.com>
Signed-off-by: 红 <wxh16144@qq.com>
@Wxh16144 Wxh16144 force-pushed the wuxh/feat-form-scrollToField-ref branch from 3180443 to 85e12ff Compare April 2, 2024 15:57
@zombieJ
Copy link
Member

zombieJ commented Apr 9, 2024

这个盖住了:

截屏2024-04-09 22 59 01

@crazyair
Copy link
Member

这个盖住了:

截屏2024-04-09 22 59 01

新的 demo 用垂直布局方便

@Wxh16144
Copy link
Member Author

这个盖住了:
截屏2024-04-09 22 59 01

新的 demo 用垂直布局方便

我回想了一下为什么从一开始的垂直布局改成水平布局,是因为垂直模式下,从下往上滑动的时候,不会吧 label 滑动进去,会有点奇怪,但是又不想在 scrollToFirstError 改成垂直居中,这样 demo 也变得复杂了。 demo 保持最简单即可~

@crazyair
Copy link
Member

这个盖住了:
截屏2024-04-09 22 59 01

新的 demo 用垂直布局方便

我回想了一下为什么从一开始的垂直布局改成水平布局,是因为垂直模式下,从下往上滑动的时候,不会吧 label 滑动进去,会有点奇怪,但是又不想在 scrollToFirstError 改成垂直居中,这样 demo 也变得复杂了。 demo 保持最简单即可~

所以滚动用 center 了,另外这也算是个小问题,应该滚动 label 的位置,没有 label 才滚动 item,item noStyle 才滚动 children

@Wxh16144
Copy link
Member Author

这个盖住了:
截屏2024-04-09 22 59 01

新的 demo 用垂直布局方便

我回想了一下为什么从一开始的垂直布局改成水平布局,是因为垂直模式下,从下往上滑动的时候,不会吧 label 滑动进去,会有点奇怪,但是又不想在 scrollToFirstError 改成垂直居中,这样 demo 也变得复杂了。 demo 保持最简单即可~

所以滚动用 center 了,另外这也算是个小问题,应该滚动 label 的位置,没有 label 才滚动 item,item noStyle 才滚动 children

应该滚动 label 的位置

这个是真做不了, 直接推荐用 scrollToFirstError={{ block: 'center' }} 好了。 2333

@afc163
Copy link
Member

afc163 commented Apr 17, 2024

rebase 一下 feature,看看 ci 会不会过。

@Wxh16144
Copy link
Member Author

看起来没啥问题,可以 merge 了~

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

7 participants