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

docs(Input): refactor, add trailing . in description, fix imports #679

Merged
merged 4 commits into from
Oct 17, 2016
Merged

docs(Input): refactor, add trailing . in description, fix imports #679

merged 4 commits into from
Oct 17, 2016

Conversation

dpkwhan
Copy link
Contributor

@dpkwhan dpkwhan commented Oct 14, 2016

  • refactor
  • add trailing . in description
  • fix imports
  • fix wrong prop in Button

…x wrong props on Button and add required prop selection to Dropdown
@codecov-io
Copy link

codecov-io commented Oct 14, 2016

Current coverage is 100% (diff: 100%)

No coverage report found for master at d215e54.

Powered by Codecov. Last update d215e54...4398632

<Input
action={<Dropdown basic floating options={options} defaultValue='page' />}
action={<Dropdown selection basic floating options={options} defaultValue='page' />}
Copy link
Member

Choose a reason for hiding this comment

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

This should not be aselection Dropdown, reference: http://semantic-ui.com/elements/input#action

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 removed selection, but it complains:

Warning: Failed prop type: `options` prop in `Dropdown` requires props: `selection`.
    in Dropdown (created by InputExampleActionDropdown)
    in InputExampleActionDropdown


const InputExampleActionLabeledButton = () => (
<Input
action={{ color: 'teal', label: 'right', icon: 'copy', content: 'Copy' }}
Copy link
Member

Choose a reason for hiding this comment

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

The prop label is for defining the label, the prop labelPosition is for placing it on the left/right. This should be labelPosition: 'right'.

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.


const InputExampleLeftActionLabeledButton = () => (
<Input
action={{ color: 'teal', label: true, icon: 'cart', content: 'Checkout' }}
Copy link
Member

Choose a reason for hiding this comment

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

The previous labeled prop became labelPosition. This should be labelPosition: 'left' which will place the icon as a label on the left.

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.

Copy link
Member

@levithomason levithomason left a comment

Choose a reason for hiding this comment

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

One more update please, then I think we can merge it.

<Input
label={<Dropdown defaultValue='.com' options={options} />}
label={<Dropdown selection defaultValue='.com' options={options} />}
Copy link
Member

Choose a reason for hiding this comment

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

The selection prop should also not be added here:

with selection

image

without selection

image

@dpkwhan
Copy link
Contributor Author

dpkwhan commented Oct 15, 2016

@levithomason removed prop selection from Dropdown. Thank you for pointing this.

@levithomason levithomason changed the title docs(Input): refactor, add trailing . in description, fix imports, fi… docs(Input): refactor, add trailing . in description, fix imports Oct 17, 2016
@levithomason levithomason merged commit 21954b1 into Semantic-Org:master Oct 17, 2016
@levithomason
Copy link
Member

Thanks much :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants