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

Jump to bottom

chore: use father #197

Merged
merged 32 commits into from Aug 27, 2020
Merged

chore: use father #197

merged 32 commits into from Aug 27, 2020

Conversation

xrkffgg
Copy link
Member

@xrkffgg xrkffgg commented Aug 19, 2020

No description provided.

@vercel
Copy link

vercel bot commented Aug 19, 2020 •
edited

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/react-component/dialog/ojgv7nz43
✅ Preview: https://dialog-git-up-father.react-component.vercel.app

@xrkffgg

This comment has been minimized.

Sign in to view
@xrkffgg

This comment has been minimized.

Sign in to view
xrkffgg
xrkffgg commented Aug 20, 2020
View reviewed changes
.gitignore Show resolved Hide resolved
@xrkffgg xrkffgg mentioned this pull request Aug 20, 2020
Closed
@zombieJ zombieJ mentioned this pull request Aug 20, 2020
Merged
14 tasks
@xrkffgg

This comment has been minimized.

Sign in to view
@xrkffgg xrkffgg marked this pull request as ready for review August 20, 2020 07:49
@zombieJ
Copy link
Member

zombieJ commented Aug 26, 2020

@kermit-xuan，帮忙看看哈~~

@kerm1it
Copy link
Member

kerm1it commented Aug 26, 2020

麻烦邀请一下到项目 @zombieJ

@zombieJ
Copy link
Member

zombieJ commented Aug 26, 2020

done

@kerm1it
Copy link
Member

kerm1it commented Aug 26, 2020

这是点击没有关闭的原因，

dialog/src/Dialog.tsx

Line 188 in d9604fa

if (Date.now() - this.openTime < 300) {

测试发现时间间隔在20多，肯定不能触发了，Mock一下就好了。

@vercel vercel bot temporarily deployed to Preview August 27, 2020 02:20 Inactive
@xrkffgg xrkffgg changed the title [WIP] chore: use father chore: use father Aug 27, 2020
@xrkffgg
Copy link
Member Author

xrkffgg commented Aug 27, 2020

  • compile 报错，build 正常

    • 移除 "@types/mocha": "~8.0.0" 就可以了 ✅

test

  • destroy on hide should unmount child components on close：input 输入没成功

    • 使用 state 更新 input 后，destroyOnClose 并不会重置 state ❌
    • simulate change ❌
    • 使用 document.getElementsByClassName 获取 input 设置 value ✅

  • mask to close：click 没有关闭弹窗

    • Date.now() 每次调用+500，满足click要求 ✅ @kermit-xuan

  • renderToBody：parentNode.parentNode

    • getDOMNode() ✅

  • getContainer：parentNode.parentNode.parentNode

  • trap focus after shift-tabbing：激活的不对

    • 使用 document.querySelectorAll ✅

  • sets transform-origin when property mousePosition is set：style 里面 没有 transformOrigin

    • style 在 ReactDOM 上没有 transformOrigin，使用 转为 html()，可看到 ✅
    • 使用 getDOMNode 获取 transformOrigin 。比 html() 简化许多。✅ ✅

  • overflow：感觉是没有渲染到 body 上，所以没有改变 overflow

    • mount(, { attachTo: document.body }) ❌
    • { attachTo: container } ✅

All done. @afc163 @zombieJ @kermit-xuan

zombieJ
zombieJ reviewed Aug 27, 2020
View reviewed changes
.eslintrc.js Outdated
"jsx-a11y/label-has-for": 0,
"jsx-a11y/label-has-associated-control": 0,
"import/no-extraneous-dependencies": 0,
"react/button-has-type": 0,
Copy link
Member

Choose a reason for hiding this comment

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

这个规则需要开一下，没有的地方补一补包括 example 的

zombieJ
zombieJ reviewed Aug 27, 2020
View reviewed changes
typings/custom.d.ts Outdated
@@ -1,14 +1,14 @@
declare module "rc-util/lib/KeyCode" {
var Ret: {ESC:any;TAB:any;};
const Ret: {ESC:any;TAB:any;};
export default Ret;
}

declare module "rc-util/lib/*" {
Copy link
Member

Choose a reason for hiding this comment

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

rc-util 用到的应该都有 ts 定义了，不需要这个了~

zombieJ
zombieJ reviewed Aug 27, 2020
View reviewed changes
typings/custom.d.ts
export default Ret;
}

declare module "rc-animate" {
var Ret: any;
const Ret: any;
Copy link
Member

Choose a reason for hiding this comment

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

这个合好后，看看有木有机会换成 rc-motion~~

@kermit-xuan

Copy link
Member

Choose a reason for hiding this comment

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

OK

zombieJ
zombieJ reviewed Aug 27, 2020
View reviewed changes
.eslintrc.js Outdated
"@typescript-eslint/no-explicit-any": 0,
"@typescript-eslint/no-empty-interface": 0,
"@typescript-eslint/no-inferrable-types": 0,
"react/no-find-dom-node": 0,
Copy link
Member

Choose a reason for hiding this comment

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

好像没看到用到 findDOMNode 的地方，这个 rule 可以删了~

@zombieJ zombieJ merged commit 94c2da1 into master Aug 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the up-father branch August 27, 2020 08:57
@afc163
Copy link
Member

afc163 commented Aug 27, 2020 

+ rc-dialog@8.2.0

@afc163 afc163 mentioned this pull request Aug 31, 2020
Merged
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kerm1it kerm1it kerm1it left review comments

@zombieJ zombieJ zombieJ approved these changes

@afc163 afc163 Awaiting requested review from afc163

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@xrkffgg @zombieJ @kerm1it @afc163