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 Checkbox and icon-only Checkbox #27

Merged
merged 17 commits into from
Apr 26, 2017
Merged

Conversation

zhusee2
Copy link
Contributor

@zhusee2 zhusee2 commented Apr 24, 2017

Purpose

Add normal <Checkbox> and icon-only <IconCheckbox>.

Implement

  • Migrate <Checkbox> with indeterminate support and clean layout
  • Add <IconCheckbox> to override checkbox button and make component minified.
  • Add tests for /index.js to check if every component is proper exported.

Demo

2017-04-25 11 31 25

iconWrapper: ROOT_BEM.element('icon-wrapper'),
};

export const CHECKBOX_BUTTON = <Icon type="empty" className={`${BEM.button}`} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

component 我還是建議用駝峰命名 XD <CheckboxButton>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

因為他不是 component XD
他只是一個變數,存著預先定義好的 element?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

當然做成這樣也可以啦

function CheckboxButton() {
    return <Icon type="empty" className={`${BEM.button}`} />;
}

只是就覺得好像沒有這個必要

Copy link
Contributor

Choose a reason for hiding this comment

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

不太建議轉成 function

🤔 🤔 🤔 沒關係就保留原本的吧


const srcPath = path.resolve(__dirname, '..');

describe('Gypcrete Bundle', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 👍 👍

@cjies
Copy link
Contributor

cjies commented Apr 26, 2017

others LGTM

@zhusee2 zhusee2 merged commit 77772fb into develop Apr 26, 2017
@zhusee2 zhusee2 deleted the feature/zhusee_checkbox branch April 26, 2017 09:44
@zhusee2 zhusee2 mentioned this pull request Apr 27, 2017
@zhusee2 zhusee2 mentioned this pull request May 24, 2017
18 tasks
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

2 participants