-
-
Notifications
You must be signed in to change notification settings - Fork 187
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: invalid autofocus #361
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
似乎原生 |
@@ -41,7 +42,7 @@ const DialogWrap: React.FC<IDialogPropTypes> = (props: IDialogPropTypes) => { | |||
<Portal | |||
open={visible || forceRender || animatedVisible} | |||
autoDestroy={false} | |||
getContainer={getContainer} | |||
getContainer={isNil(getContainer) ? 'body' : getContainer} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be a necessary change, Portal
will check it's correct internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rc-potral默认的挂载点似乎不太稳定,有可能虚拟dom渲染的时候挂载点还没添加到页面,或者添加后又被移除(visible为false的情况),导致autofocus失效。
这里相当于改用body作为默认挂载点。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那是不是分两个问题修复? 后者在 Portal 修复?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
portal
好像不太好修,可以把这个当作临时方案。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zombieJ 呼叫巨佬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我也觉得这是两件事。这个 PR 应该只管 Input focus 的问题。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
也许得先优化一下portal
https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js#L522-L528
看出react的autofocus是用js模拟的,如何让react在https://github.com/react-component/portal/blob/master/src/useDom.tsx#L68 append之后模拟autofocus?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
拉你进群了,这种需要沟通比较多的问题到群里沟通吧
Codecov Report
@@ Coverage Diff @@
## master #361 +/- ##
==========================================
- Coverage 98.12% 97.61% -0.51%
==========================================
Files 7 7
Lines 160 168 +8
Branches 50 54 +4
==========================================
+ Hits 157 164 +7
- Misses 2 3 +1
Partials 1 1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@acyza 加个群? |
Hi, any news for this PR? |
fix ant-design/ant-design#41239
link #350
react中的
autoFocus
似乎是react模拟的,并非原生的autofocus。https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js#L522-L528rc-potral
默认的挂载点似乎不太稳定,有可能虚拟dom渲染的时候挂载点还没添加到页面,或者添加后又被移除(visible
为false的情况),导致autofocus
失效。