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.

Sign up for GitHub

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

Jump to bottom

fix(components): [select] optimize the triggering of blur event #11524

Merged
merged 4 commits into from
May 28, 2023
Merged

fix(components): [select] optimize the triggering of blur event #11524

merged 4 commits into from
May 28, 2023

Conversation

tolking
Copy link
Member

@tolking tolking commented Feb 12, 2023 •
edited

fix #10893, fix #10717, closed #10992

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow contributing guide English | (中文 | Español | Français).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer to relative issues for your PR.

At present, the handleBlur function determines whether to trigger the blur event through isSilentBlur, and the isSilentBlur is modified by clicking Option to trigger the handleOptionSelect function.

However, the handleOptionSelect function is triggered later than the handleBlur function, the blur event may be triggered incorrectly.

This PR modified to: After the focus transferred from input, judge whether it is in the drop-down selection. same PR #10091

before

after

@github-actions
Copy link

github-actions bot commented Feb 12, 2023 •
edited

@github-actions github-actions bot added the CommitMessage::Qualified Qualified commit message label Feb 12, 2023
@github-actions
Copy link

github-actions bot commented Feb 12, 2023 •
edited

🧪 Playground Preview: https://element-plus.run/?pr=11524
Please comment the example via this playground if needed.

@chenxch
Copy link
Member

chenxch commented Feb 12, 2023

The drop-down box pops up again when it is closed.

@tolking tolking changed the title fix(components): [select] drop-down selection is not closed when blur WIP: fix(components): [select] drop-down selection is not closed when blur Feb 12, 2023
@tolking tolking changed the title WIP: fix(components): [select] drop-down selection is not closed when blur fix(components): [select] optimize the triggering of blur event Feb 13, 2023
@acyza
Copy link
Contributor

acyza commented Feb 13, 2023 •
edited

#10893 的问题还在
他这个bug可能是因为ElMessage弹出焦点被ElMessage中的ElFocusTrap改变，按OK按钮ElMessage被清除，ElFocusTrap把焦点返回到了还在Eldrawer关闭动画的ElSelect触发了ElSelect的onfocus打开下拉列表。

@tolking
Copy link
Member Author

tolking commented Feb 13, 2023 •
edited

#10893 的问题还在 他这个bug可能是因为ElMessage弹出焦点被ElMessage中的ElFocusTrap改变，按OK按钮ElMessage被清除，ElFocusTrap把焦点返回到了还在Eldrawer关闭动画的ElSelect触发了ElSelect的onfocus打开下拉列表。

你确定问题依然存在，我怎么感觉已经修复了 demo

ElFocusTrap 只会捕获焦点。在 ElMessage 关闭时 ElFocusTrap 将焦点移动到上一个聚焦的地方(ElSelect)，同理 Eldrawer 关闭时焦点也变了，触发 ElSelect 的 blur 事件

@acyza
Copy link
Contributor

acyza commented Feb 13, 2023 •
edited

代码从fix/select-blur分支迁出，App.vue用的#10893 中的demo。
firefox的效果

2023-02-13.20-11-34.mp4

chrome的效果 因为只是弹出一下后消失，因此放慢了动画。

2023-02-13.20-13-16.mp4

@acyza
Copy link
Contributor

acyza commented Feb 13, 2023 •
edited

不是ElMessageBox中ElFocusTrap的问题，我直接把ElMessageBox中的ElFocusTrap去掉，用js代码手动把焦点导到ElMessageBox的按钮上，点击OK按钮后ElSelect还是弹出来了。

@tolking
Copy link
Member Author

tolking commented Feb 13, 2023

chrome 这样是符合设计的

我 Safari 也复现了火狐的问题，弹窗没有被关闭。

简单排查了下，像是 button 无法被聚焦引起的，原生 button Safari 都没有聚焦。

麻烦你测试下火狐是不是也是这种情况（点页面空白处，然后使用 Tab 按键）

@acyza
Copy link
Contributor

acyza commented Feb 13, 2023

2023-02-13.21-09-35.mp4

这样吗？看着好像没啥问题，按钮都能聚集啊！

@acyza
Copy link
Contributor

acyza commented Feb 13, 2023 •
edited

https://github.com/element-plus/element-plus/blob/dev/packages/components/message-box/src/index.vue#L476
ElSelect的焦点是通过这里的useRestoreActive回去的。
或者说ElFocusTrap并没有退回焦点。

@tolking
Copy link
Member Author

tolking commented Feb 13, 2023

chrome 都成功了。可能是使用了具有差异浏览器api？

@acyza
Copy link
Contributor

acyza commented Feb 13, 2023

chrome 都成功了。可能是使用了具有差异浏览器api？

我测了下chrome在visibility: hidden;会触发blur，但firefox不会，会不会是这个原因？

@acyza acyza mentioned this pull request Feb 13, 2023
Closed
3 tasks
@tolking
Copy link
Member Author

tolking commented Feb 14, 2023

这应该是 FocusTrap 的问题。从2.2.18后，当使用鼠标关闭 Dialog、Drawer 弹窗时，焦点将无法返回之前的按钮。

另外我觉得应该由 FocusTrap 控制焦点更合理

@tolking tolking changed the title fix(components): [select] optimize the triggering of blur event WIP: fix(components): [select] optimize the triggering of blur event Feb 17, 2023
@tolking tolking changed the title WIP: fix(components): [select] optimize the triggering of blur event fix(components): [select] optimize the triggering of blur event Feb 18, 2023
acyza
acyza reviewed Feb 18, 2023
View reviewed changes
Copy link
Contributor

@acyza acyza left a comment

Choose a reason for hiding this comment

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

LGTM

@tolking tolking linked an issue Apr 4, 2023 that may be closed by this pull request
[Component] [select] select 选择选项后触发blur #12318
Closed
@tolking tolking mentioned this pull request May 1, 2023
Closed
3 tasks
@btea btea merged commit a99dbb3 into dev May 28, 2023
@btea btea deleted the fix/select-blur branch May 28, 2023 09:31
@element-bot element-bot mentioned this pull request Jun 2, 2023
Merged
3 tasks
@adrkn adrkn mentioned this pull request Jun 7, 2023
Closed
This was referenced Jul 6, 2023
Closed
Closed
@EthanLuu EthanLuu mentioned this pull request Jul 21, 2023
Closed
consultation-applio pushed a commit to consultation-applio/element-plus that referenced this pull request Nov 10, 2023
@tolking @dannyhebertbell
fix(components): [select] optimize the triggering of blur event (elem…
b71fefb 
…ent-plus#11524)

* fix(components): [select] drop-down selection is not closed when blur

closed element-plus#10893, element-plus#10992

* fix(components): [select] optimize the triggering of blur event

* refactor(components): [select] optimize code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acyza acyza acyza left review comments

@chenxch chenxch chenxch approved these changes

@btea btea btea approved these changes

Assignees

@tolking tolking

Labels
CommitMessage::Qualified Qualified commit message
Projects
None yet
Milestone
No milestone
4 participants
@tolking @chenxch @acyza @btea