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

feat(dialog): add autofocus to resolve open dialog again when pressing Enter Key #1586

Merged
merged 13 commits into from
Dec 16, 2021
Merged

feat(dialog): add autofocus to resolve open dialog again when pressing Enter Key #1586

merged 13 commits into from
Dec 16, 2021

Conversation

LYErin
Copy link
Contributor

@LYErin LYErin commented Nov 15, 2021

close #1559

@vercel
Copy link

vercel bot commented Nov 15, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tusimple/naive-ui/DxhvAAH7xotonio85fhyK8DSPbgj
✅ Preview: https://naive-ui-git-fork-lyerin-fix-dialog-focus-button-tusimple.vercel.app

[Deployment for 59494f6 failed]

@codecov
Copy link

codecov bot commented Nov 15, 2021 •
edited
Loading

Codecov Report

Merging #1586 (2eaf652) into main (08ea08f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1586   +/-   ##
=======================================
  Coverage   64.79%   64.80%           
=======================================
  Files         890      890           
  Lines       17297    17301    +4     
  Branches     4098     4099    +1     
=======================================
+ Hits        11208    11212    +4     
  Misses       5313     5313           
  Partials      776      776
Impacted Files Coverage Δ
src/dialog/src/Dialog.tsx 77.77% <ø> (ø)
src/button/src/Button.tsx 65.60% <100.00%> (+0.74%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08ea08f...2eaf652. Read the comment docs.

07akioni
07akioni reviewed Nov 15, 2021
View reviewed changes
src/button/src/Button.tsx Outdated
@@ -142,11 +148,6 @@ const Button = defineComponent({
return
}
enterPressedRef.value = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

后面那个为啥去掉了？感觉好像是为了模拟点击的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

后面那个为啥去掉了？感觉好像是为了模拟点击的

加了这个 按enter的时候 会执行两次click事件; 移除就是正常的一次触发

src/button/src/Button.tsx Outdated Show resolved Hide resolved
src/dialog/src/Dialog.tsx Outdated Show resolved Hide resolved
@07akioni
Copy link
Collaborator

这块怎么做我再斟酌一下，不是很确定现在这样实现的方式是最合适的

@07akioni 07akioni merged commit 11db4be into tusen-ai:main Dec 16, 2021
07akioni
07akioni reviewed Dec 16, 2021
View reviewed changes
src/dialog/src/Dialog.tsx
@@ -267,6 +268,7 @@ export default defineComponent({
themeOverrides={mergedTheme.peerOverrides.Button}
disabled={loading}
loading={loading}
internalAutoFocus={!this.negativeText}
Copy link
Collaborator

@07akioni 07akioni Dec 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

这两行有点问题的，应该是只要显示确定按钮就 focus 确定按钮，现在反了，只要展示了不确认就 focus 不确认

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@07akioni 07akioni 07akioni left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dialog component does not give focus to action button
2 participants
@LYErin @07akioni