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

Add promise-like interface #10470

Merged
merged 16 commits into from May 25, 2018
Merged

Conversation

zhujinxuan
Copy link
Contributor

@zhujinxuan zhujinxuan commented May 11, 2018

Description

#10421

This PR provides the then(onClose) promise-like interface for the message

Check List

Please makes sure that these checkboxes are checked before submitting your PR, thank you!

  • Make sure that you propose PR to right branch: bugfix for master, feature for latest active branch feature-x.x.
  • Make sure that you follow antd's code convention.
  • Run npm run lint and fix those errors before submitting in order to keep consistent code style.
  • Rebase before creating a PR to keep commit history clear.
  • Add some descriptions and refer relative issues for you PR.

Extra checklist:

elif isNewFeature :

  • Update API docs for the component.
  • Update Demo for new feature
  • Update TypeScript definition for the component.
  • Add unit tests for the feature.

@zhujinxuan zhujinxuan changed the title Add promise-like interface [On working] Add promise-like interface May 11, 2018
@ant-design-bot
Copy link
Contributor

ant-design-bot commented May 11, 2018

Deploy preview for ant-design ready!

Built with commit f5fe409

https://deploy-preview-10470--ant-design.netlify.com

@codecov
Copy link

codecov bot commented May 11, 2018

Codecov Report

Merging #10470 into feature-3.6.0 will increase coverage by 1.34%.
The diff coverage is 100%.

Impacted file tree graph

@@                Coverage Diff                @@
##           feature-3.6.0   #10470      +/-   ##
=================================================
+ Coverage          86.51%   87.85%   +1.34%     
=================================================
  Files                196      194       -2     
  Lines               4798     4760      -38     
  Branches            1340     1329      -11     
=================================================
+ Hits                4151     4182      +31     
+ Misses               644      574      -70     
- Partials               3        4       +1
Impacted Files Coverage Δ
components/message/index.tsx 86.44% <100%> (+2.12%) ⬆️
components/index.tsx 42.85% <0%> (-57.15%) ⬇️
components/spin/index.tsx 77.55% <0%> (-1.7%) ⬇️
components/_util/openAnimation.tsx 68.18% <0%> (-1.39%) ⬇️
components/anchor/Anchor.tsx 72.72% <0%> (-0.23%) ⬇️
components/_util/throttleByAnimationFrame.tsx 95.23% <0%> (-0.22%) ⬇️
components/back-top/index.tsx 91.37% <0%> (-0.15%) ⬇️
components/pagination/Pagination.tsx 100% <0%> (ø) ⬆️
components/input-number/index.tsx 100% <0%> (ø) ⬆️
components/cascader/index.tsx 74.66% <0%> (ø) ⬆️
... and 14 more

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 f3545cc...f5fe409. Read the comment docs.

@zhujinxuan zhujinxuan changed the title [On working] Add promise-like interface [Working on Document] Add promise-like interface May 11, 2018
@zhujinxuan zhujinxuan changed the title [Working on Document] Add promise-like interface Add promise-like interface May 11, 2018
</div>
),
onClose,
const closePromise = new Promise((resolve) => {
Copy link
Member

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.

I am not sure. Do you mean some browsers ant-design supporting does not support Promise?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's ok, never mind.

// calculate the approximately duration value
const aboutDuration = parseInt((Date.now() - now) / 1000, 10);
expect(aboutDuration).toBe(defaultDuration);
done();
});
}).then(() => true);
});
Copy link
Member

Choose a reason for hiding this comment

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

Write a separator test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thank you.

@@ -35,6 +35,12 @@ title: Message
- `message.config(options)`
- `message.destroy()`

组件同时提供 then 接口
Copy link
Member

Choose a reason for hiding this comment

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

then 改成 promise,后面加句号。

Copy link
Member

Choose a reason for hiding this comment

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

加一个空行。

@@ -35,6 +35,12 @@ title: Message
- `message.config(options)`
- `message.destroy()`

组件同时提供 then 接口
- `message[level](content, [duration] ).then(afterClose)`
- `message[level](content, [duration], onClose ).then(afterClose)`
Copy link
Member

Choose a reason for hiding this comment

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

onClose 后面多余空格。

- `message[level](content, [duration] ).then(afterClose)`
- `message[level](content, [duration], onClose ).then(afterClose)`

其中`message[level]` 是组件已经提供的静态方法。`then` 接口返回值是 Promise
Copy link
Member

Choose a reason for hiding this comment

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

`message 前端少了一个空格。

Copy link
Member

Choose a reason for hiding this comment

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

句尾加标点。

@afc163
Copy link
Member

afc163 commented May 23, 2018

再补充一下用例吧。

@zhujinxuan
Copy link
Contributor Author

@afc163 请问建议补充怎样的用例呢?

@afc163
Copy link
Member

afc163 commented May 23, 2018

测试 promise 用法有效并符合预期。

@zhujinxuan
Copy link
Contributor Author

@afc163 请问你指的是这个 PR 中的 components/message/demo/thenable.md 么?
我刚刚修改了这个的文档,让这个文档更加明了一些

@@ -19,7 +19,7 @@ import { message, Button } from 'antd';
const success = () => {
const hide = message.loading('Action in progress..', 0);
// Dismiss manually and asynchronously
setTimeout(hide, 2500);
setTimeout(hide, 500);
Copy link
Member

Choose a reason for hiding this comment

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

No need to change this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I revert the change in recent commit 799bcfb. Thank you.

@afc163 afc163 merged commit c2519e7 into ant-design:feature-3.6.0 May 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants