Skip to content
This repository has been archived by the owner on Oct 20, 2021. It is now read-only.

feat(Spinner/Switch): add spinner and switch components #30

Merged
merged 14 commits into from
Nov 21, 2018
Merged

feat(Spinner/Switch): add spinner and switch components #30

merged 14 commits into from
Nov 21, 2018

Conversation

Kimi-Gao
Copy link
Member

@Kimi-Gao Kimi-Gao commented Nov 20, 2018 •
edited

First of all, thank you for your contribution! :-)

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 earth-ui's code convention.
  • Add some descriptions and refer relative issues for you PR.

Extra checklist:

elif isNewFeature :

  • Update API docs for the component.
  • Update/Add demo to demonstrate new feature.
  • Add unit tests for the feature.

Description:

  1. Add Spinner and Switch components.
  2. Move docs to each component dir.
  3. Optimize Board components.
  4. Optimize Demo in site.
  5. Upgrade react/react-dom to v16
  6. Remove bootcdn(cuz unstable sometimes) and use local libs.
  7. Other small tweaks...

ScreenShots:

2018-11-20 2 24 36

@Kimi-Gao Kimi-Gao self-assigned this Nov 20, 2018
@Kimi-Gao Kimi-Gao added PR: refactor A code change that neither fixes a bug nor adds a feature PR: feat A new feature PR: build Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm) PR: docs Documentation only changes labels Nov 20, 2018
@Kimi-Gao Kimi-Gao changed the title Spinner feat(Spinner/Switch): add spinner and switch components Nov 20, 2018
@goroyal
Copy link
Member

goroyal commented Nov 20, 2018

怎么一下子这么多修改？

goroyal
goroyal reviewed Nov 21, 2018
View reviewed changes
src/components/Spinner/index.js
@@ -13,18 +13,31 @@ const Spinner = props => {
}

return (
<div className={cx(`${prefixCls}-spinner`, className)} {...other}>
Copy link
Member

Choose a reason for hiding this comment

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

为什么此处会夹一段HTML的代码？

Copy link
Member Author

Choose a reason for hiding this comment

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

这个是react的JSX语法

@goroyal
Copy link
Member

goroyal commented Nov 21, 2018

尽量频繁提小PR啊

@Kimi-Gao
Copy link
Member Author

@goroyal 本来只想写一个Spinner 后来越写要改的地方越多 慢慢scope就扩大了

@Kimi-Gao
Copy link
Member Author

@goroyal 另外是项目很多地方需要完善 还不稳定

@goroyal
Copy link
Member

goroyal commented Nov 21, 2018

@goroyal 另外是项目很多地方需要完善 还不稳定

好吧

@goroyal
Copy link
Member

goroyal commented Nov 21, 2018

需要2个人approve？

@Kimi-Gao
Copy link
Member Author

@goroyal 至少一个 可能是你approve的方式有问题

goroyal
goroyal approved these changes Nov 21, 2018
View reviewed changes
Copy link
Member

@goroyal goroyal left a comment

Choose a reason for hiding this comment

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

That's OK.

@Kimi-Gao Kimi-Gao merged commit e259a52 into feature-0.3 Nov 21, 2018
@Kimi-Gao Kimi-Gao deleted the spinner branch November 21, 2018 02:52
@Kimi-Gao Kimi-Gao removed PR: build Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm) PR: docs Documentation only changes PR: refactor A code change that neither fixes a bug nor adds a feature labels Sep 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@goroyal goroyal goroyal approved these changes

@ice418 ice418 Awaiting requested review from ice418

@weiyuxuan weiyuxuan Awaiting requested review from weiyuxuan

Assignees

@Kimi-Gao Kimi-Gao

Labels
PR: feat A new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Kimi-Gao @goroyal