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: Upload accessibility experience #46432

Merged
merged 3 commits into from Dec 30, 2023
Merged

fix: Upload accessibility experience #46432

merged 3 commits into from Dec 30, 2023

Conversation

afc163
Copy link
Member

@afc163 afc163 commented Dec 13, 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

close #46215
close #24771
close #45922

💡 Background and solution

修复 Upload 包裹 Button 时需要 tab 两次的问题。基于 react-component/upload#521

  • 在 antd 这里默认开启了 hasControlInside 因为大多数时候是和 Button 一起配合使用。
  • 在 Dragger 内默认关闭了 hasControlInside,因为拖拽上传的 children 一般不是 Button。
  • 先不在文档中透出 hasControlInside,因为这个属性比较难理解,一般无需关注。

📝 Changelog

Language Changelog
🇺🇸 English Fix Upload accessibility issue of tabbing twicely.
🇨🇳 Chinese 修复 Upload 组件包裹 Button 时 Tab 键会聚焦两次的问题。

☑️ 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 Dec 13, 2023

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

Copy link
Contributor

github-actions bot commented Dec 13, 2023

Visual Regression Report for PR #46432 Failed ❌

Target branch: master (44d1b68)
View Full Report


image name expected actual diff
form-validate-other.png master: 44d1b680b8b0432ea8272a0a6c96b2b0e7cab1ab current: pr-46432 diff
form-validate-other.css-var.png master: 44d1b680b8b0432ea8272a0a6c96b2b0e7cab1ab current: pr-46432 diff
upload-drag.png master: 44d1b680b8b0432ea8272a0a6c96b2b0e7cab1ab current: pr-46432 diff
upload-drag.css-var.png master: 44d1b680b8b0432ea8272a0a6c96b2b0e7cab1ab current: pr-46432 diff

Check Full Report for details

Copy link
Contributor

github-actions bot commented Dec 13, 2023

Preview Is ready

Copy link

socket-security bot commented Dec 13, 2023

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
rc-upload 4.5.2 None +0 66.7 kB zombiej

Copy link
Contributor

github-actions bot commented Dec 13, 2023

size-limit report 📦

Path Size
./dist/antd.min.js 328.78 KB (+68 B 🔺)
./dist/antd-with-locales.min.js 374.73 KB (+41 B 🔺)

@afc163 afc163 changed the title fix: Upload tab experience fix: Upload accessibility experience Dec 13, 2023
@afc163 afc163 marked this pull request as ready for review December 13, 2023 11:15
Copy link

codesandbox-ci bot commented Dec 13, 2023

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 cfe8085:

Sandbox Source
antd reproduction template (forked) Configuration
antd reproduction template (forked) Issue #45922

Copy link

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (44d1b68) 100.00% compared to head (cfe8085) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #46432   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          708       708           
  Lines        12120     12122    +2     
  Branches      3218      3220    +2     
=========================================
+ Hits         12120     12122    +2     

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

@zombieJ
Copy link
Member

zombieJ commented Dec 13, 2023

有点猜的味道,是不是可以优化成监听 focus 事件,在 focus 时看内部是否有可以聚焦的元素,如果有的话就 proxy 到子元素中。用这个方法:
getFocusNodeList

@yoyo837
Copy link
Contributor

yoyo837 commented Dec 13, 2023

需要添加 react-component/upload#518 的文档

@afc163 afc163 closed this Dec 14, 2023
@afc163 afc163 reopened this Dec 14, 2023
@afc163
Copy link
Member Author

afc163 commented Dec 14, 2023

有点猜的味道,是不是可以优化成监听 focus 事件,在 focus 时看内部是否有可以聚焦的元素,如果有的话就 proxy 到子元素中。用这个方法:

这个方式有 DOM 查询的副作用,不太想这么搞。

@zombieJ
Copy link
Member

zombieJ commented Dec 22, 2023

这个方式有 DOM 查询的副作用,不太想这么搞。

focus 事件触发去查是惰性的,不过在修那个 button 触发两次的事件后,我发现 getFocusNodeList 不行。因为一个是故意挪到 upload div 上,一个是挪到内部 button 上。两个相冲了。

@li-jia-nan
Copy link
Member

Conflicting

@afc163 afc163 merged commit 8705640 into master Dec 30, 2023
105 of 106 checks passed
@afc163 afc163 deleted the fix/upload-tabindex branch December 30, 2023 07:21
Copy link
Contributor

🎉 Thank you for your contribution! If you have not yet joined our DingTalk community group, please feel free to join us (when joining, please provide the link to this PR).

🎉 感谢您的贡献!如果您还没有加入钉钉社区群,请扫描下方二维码加入我们(加群时请提供此 PR 链接)。

@FSbrand
Copy link

FSbrand commented Jan 5, 2024

ant-upload-drag由display:table改为了display:flex,这样做的原因是什么,突然发现页面的样式被改变

@yoyo837
Copy link
Contributor

yoyo837 commented Jan 5, 2024

ant-upload-drag由display:table改为了display:flex,这样做的原因是什么,突然发现页面的样式被改变

#46810 改回去了,等发布

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants