-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(Item.Image): Add size prop #747
feat(Item.Image): Add size prop #747
Conversation
clemensw
commented
Oct 25, 2016
- Resolves Item.Image doesn't honor size prop #746
- Reverses an explicit test for ui=false, so needs review.
- Renders similar to the example at http://semantic-ui.com/views/item.html#image
- Images now render left aligned and sized.
Current coverage is 99.48% (diff: 100%)
|
32053f9
to
df4dc82
Compare
This PR will break all other except case with For a common understanding, we are talking about the example. This example uses So, we need add <Item.Image /> // <div class="ui image">
<Item.Image size="small" /> // <div class="ui small image"> I think it's not responsibility of - import React from 'react'
+ import React, { PropTypes } from 'react'
import {
META,
+ SUI,
} from '../../lib'
import Image from '../../elements/Image'
function ItemImage(props) {
- return <Image {...props} ui={false} wrapped />
+ const { size } = props
+ const rest = getUnhandledProps(ItemImage, props)
+
+ return <Image {...props} size={size} ui={!!size} wrapped />
}
ItemImage._meta = {
name: 'ItemImage',
parent: 'Item',
type: META.TYPES.VIEW,
}
+ ItemImage.propTypes = {
+ /** An image may appear at different sizes */
+ size: PropTypes.oneOf(Image._meta.props.size),
+ },
export default ItemImage import React from 'react'
import ItemImage from 'src/views/Item/ItemImage'
describe('ItemImage', () => {
it('renders Image component', () => {
shallow(<ItemImage />)
.should.have.descendants('Image')
})
it('is wrapped without ui', () => {
const wrapper = shallow(<ItemImage />)
wrapper.should.have.prop('wrapped', true)
wrapper.should.have.prop('ui', false)
})
+ it('has ui with size prop', () => {
+ shallow(<ItemImage size='small' />)
+ .should.have.prop('ui', true)
+ })
}) |
Let's also use |
@clemensw do you have plans for PR update? |
Would like to merge @clemensw's work, however, whoever has time to get to this first let's ship it! :) |
df4dc82
to
21d1515
Compare
Thank you for the review. I'm seriously confused by the logic behind Semantic UI 's use of the ui class in css. I've updated the PR based on your suggestions (substituting getUnhandledProps for SUI on the import and rest for props on the Image attributes). I booleanized size by checking against undefined, but will re-push the PR with your solution. |
…y to work * Resolves Semantic-Org#746 * Add ui class only for sized images * Renders similar to the example at http://semantic-ui.com/views/item.html#image * Images now render left aligned and sized.
21d1515
to
3f0533c
Compare
@clemensw Thanks for updates 😺 |
Thanks for the update and help! |