feat(Image): improve handling of HTML props (#2316)

* feat(Image): improve handling of HTML props

* style(mixed): rename function to partitionHTMLProps

docs/app/Examples/elements/Image/Usage/ImageExampleImageProps.js

@@ -0,0 +1,13 @@
+import React from 'react'
+import { Image } from 'semantic-ui-react'
+
+const ImageExampleImageProps = () => (
+  <Image
+    alt='An example alt'
+    size='small'
+    src='/assets/images/wireframe/image-text.png'
+    srcSet='/assets/images/wireframe/image.png 2x'
+  />
+)
+
+export default ImageExampleImageProps

docs/app/Examples/elements/Image/Usage/ImageExampleWrappedImageProps.js

@@ -0,0 +1,14 @@
+import React from 'react'
+import { Image } from 'semantic-ui-react'
+
+const ImageExampleWrappedImageProps = () => (
+  <Image
+    as='div'
+    alt='An example alt'
+    size='small'
+    src='/assets/images/wireframe/image-text.png'
+    srcSet='/assets/images/wireframe/image.png 2x'
+  />
+)
+
+export default ImageExampleWrappedImageProps

docs/app/Examples/elements/Image/Usage/index.js

@@ -1,4 +1,5 @@
 import React from 'react'
+
 import ComponentExample from 'docs/app/Components/ComponentDoc/ComponentExample'
 import ExampleSection from 'docs/app/Components/ComponentDoc/ExampleSection'
 
@@ -9,6 +10,15 @@ const ImageUsageExamples = () => (
       description='An image can render children.'
       examplePath='elements/Image/Usage/ImageExampleChildren'
     />
+    <ComponentExample
+      title='Image props'
+      description={<span>An image correctly handles props of an HTML <code>img</code>.</span>}
+      examplePath='elements/Image/Usage/ImageExampleImageProps'
+    />
+    <ComponentExample
+      description='Also when it is wrapped in an another element.'
+      examplePath='elements/Image/Usage/ImageExampleWrappedImageProps'
+    />
   </ExampleSection>
 )
 

src/elements/Image/Image.d.ts

@@ -18,9 +18,6 @@ export interface ImageProps {
   /** An element type to render as (string or function). */
   as?: any;
 
-  /** Alternate text for the image specified. */
-  alt?: string;
-
   /** An image may be formatted to appear inline with text as an avatar. */
   avatar?: boolean;
 
@@ -54,9 +51,6 @@ export interface ImageProps {
   /** An image can take up the width of its container. */
   fluid?: boolean;
 
-  /** The img element height attribute. */
-  height?: string | number;
-
   /** An image can be hidden. */
   hidden?: boolean;
 
@@ -76,20 +70,14 @@ export interface ImageProps {
   size?: SemanticSIZES;
 
   /** An image can specify that it needs an additional spacing to separate it from nearby content. */
-  spaced?: boolean|'left'|'right';
-
-  /** Specifies the URL of the image. */
-  src?: string;
+  spaced?: boolean | 'left' | 'right';
 
   /** Whether or not to add the ui className. */
   ui?: boolean;
 
   /** An image can specify its vertical alignment. */
   verticalAlign?: SemanticVERTICALALIGNMENTS;
 
-  /** The img element width attribute. */
-  width?: string | number;
-
   /** An image can render wrapped in a `div.ui.image` as alternative HTML markup. */
   wrapped?: boolean;
 }

src/elements/Image/Image.js

@@ -10,6 +10,7 @@ import {
   getElementType,
   getUnhandledProps,
   META,
+  partitionHTMLProps,
   SUI,
   useKeyOnly,
   useKeyOrValueAndKey,
@@ -18,16 +19,16 @@ import {
 } from '../../lib'
 import Dimmer from '../../modules/Dimmer'
 import Label from '../Label/Label'
-
 import ImageGroup from './ImageGroup'
 
+const imageProps = ['alt', 'height', 'src', 'srcSet', 'width']
+
 /**
  * An image is a graphic representation of something.
  * @see Icon
  */
 function Image(props) {
   const {
-    alt,
     avatar,
     bordered,
     centered,
@@ -39,17 +40,14 @@ function Image(props) {
     disabled,
     floated,
     fluid,
-    height,
     hidden,
     href,
     inline,
     label,
     rounded,
     size,
     spaced,
-    src,
     verticalAlign,
-    width,
     wrapped,
     ui,
   } = props
@@ -73,6 +71,7 @@ function Image(props) {
     className,
   )
   const rest = getUnhandledProps(Image, props)
+  const [imgTagProps, rootProps] = partitionHTMLProps(rest, { htmlProps: imageProps })
   const ElementType = getElementType(Image, props, () => {
     if (!_.isNil(dimmer) || !_.isNil(label) || !_.isNil(wrapped) || !childrenUtils.isNil(children)) return 'div'
   })
@@ -84,13 +83,9 @@ function Image(props) {
     return <ElementType {...rest} className={classes}>{content}</ElementType>
   }
 
-  const rootProps = { ...rest, className: classes }
-  const imgTagProps = { alt, src, height, width }
-
-  if (ElementType === 'img') return <ElementType {...rootProps} {...imgTagProps} />
-
+  if (ElementType === 'img') return <ElementType {...rootProps} {...imgTagProps} className={classes} />
   return (
-    <ElementType {...rootProps} href={href}>
+    <ElementType {...rootProps} className={classes} href={href}>
       {Dimmer.create(dimmer)}
       {Label.create(label)}
       <img {...imgTagProps} />
@@ -109,9 +104,6 @@ Image.propTypes = {
   /** An element type to render as (string or function). */
   as: customPropTypes.as,
 
-  /** Alternate text for the image specified. */
-  alt: PropTypes.string,
-
   /** An image may be formatted to appear inline with text as an avatar. */
   avatar: PropTypes.bool,
 
@@ -148,12 +140,6 @@ Image.propTypes = {
     customPropTypes.disallow(['size']),
   ]),
 
-  /** The img element height attribute. */
-  height: PropTypes.oneOfType([
-    PropTypes.number,
-    PropTypes.string,
-  ]),
-
   /** An image can be hidden. */
   hidden: PropTypes.bool,
 
@@ -178,21 +164,12 @@ Image.propTypes = {
     PropTypes.oneOf(['left', 'right']),
   ]),
 
-  /** Specifies the URL of the image. */
-  src: PropTypes.string,
-
   /** Whether or not to add the ui className. */
   ui: PropTypes.bool,
 
   /** An image can specify its vertical alignment. */
   verticalAlign: PropTypes.oneOf(SUI.VERTICAL_ALIGNMENTS),
 
-  /** The img element width attribute. */
-  width: PropTypes.oneOfType([
-    PropTypes.string,
-    PropTypes.number,
-  ]),
-
   /** An image can render wrapped in a `div.ui.image` as alternative HTML markup. */
   wrapped: PropTypes.bool,
 }

src/elements/Input/Input.js

@@ -11,7 +11,7 @@ import {
   getElementType,
   getUnhandledProps,
   META,
-  partitionHTMLInputProps,
+  partitionHTMLProps,
   SUI,
   useKeyOnly,
   useValueAndKey,
@@ -154,7 +154,7 @@ class Input extends Component {
 
     const tabIndex = this.computeTabIndex()
     const unhandled = getUnhandledProps(Input, this.props)
-    const [htmlInputProps, rest] = partitionHTMLInputProps(unhandled)
+    const [htmlInputProps, rest] = partitionHTMLProps(unhandled)
 
     return [{
       ...htmlInputProps,

src/lib/htmlInputPropsUtils.js → src/lib/htmlPropsUtils.js

@@ -43,7 +43,7 @@ export const htmlInputProps = [...htmlInputAttrs, ...htmlInputEvents]
  * @param {boolean} [options.includeAria] Includes all input props that starts with "aria-"
  * @returns {[{}, {}]} An array of objects
  */
-export const partitionHTMLInputProps = (props, options = {}) => {
+export const partitionHTMLProps = (props, options = {}) => {
   const {
     htmlProps = htmlInputProps,
     includeAria = true,

src/lib/index.js

@@ -29,8 +29,8 @@ export {
   htmlInputAttrs,
   htmlInputEvents,
   htmlInputProps,
-  partitionHTMLInputProps,
-} from './htmlInputPropsUtils'
+  partitionHTMLProps,
+} from './htmlPropsUtils'
 
 export { default as isBrowser } from './isBrowser'
 export { default as leven } from './leven'

src/modules/Checkbox/Checkbox.js

@@ -12,7 +12,7 @@ import {
   htmlInputAttrs,
   makeDebugger,
   META,
-  partitionHTMLInputProps,
+  partitionHTMLProps,
   useKeyOnly,
 } from '../../lib'
 
@@ -217,7 +217,7 @@ export default class Checkbox extends Component {
     )
     const unhandled = getUnhandledProps(Checkbox, this.props)
     const ElementType = getElementType(Checkbox, this.props)
-    const [htmlInputProps, rest] = partitionHTMLInputProps(unhandled, { htmlProps: htmlInputAttrs })
+    const [htmlInputProps, rest] = partitionHTMLProps(unhandled, { htmlProps: htmlInputAttrs })
     const id = _.get(htmlInputProps, 'id')
 
     return (

src/modules/Search/Search.js

@@ -15,7 +15,7 @@ import {
   makeDebugger,
   META,
   objectDiff,
-  partitionHTMLInputProps,
+  partitionHTMLProps,
   shallowEqual,
   SUI,
   useKeyOnly,
@@ -643,7 +643,7 @@ export default class Search extends Component {
     )
     const unhandled = getUnhandledProps(Search, this.props)
     const ElementType = getElementType(Search, this.props)
-    const [htmlInputProps, rest] = partitionHTMLInputProps(unhandled, {
+    const [htmlInputProps, rest] = partitionHTMLProps(unhandled, {
       htmlProps: htmlInputAttrs,
     })
 

test/specs/elements/Image/Image-test.js

@@ -38,10 +38,12 @@ describe('Image', () => {
 
   common.propValueOnlyToClassName(Image, 'size', SUI.SIZES)
 
-  it('renders an img tag', () => {
-    shallow(<Image />)
-      .type()
-      .should.equal('img')
+  describe('as', () => {
+    it('renders an img tag', () => {
+      shallow(<Image />)
+        .type()
+        .should.equal('img')
+    })
   })
 
   describe('href', () => {
@@ -52,6 +54,23 @@ describe('Image', () => {
     })
   })
 
+  describe('image props', () => {
+    _.forEach(['alt', 'height', 'src', 'srcSet', 'width'], (propName) => {
+      it(`keeps "${propName}" on root element by default`, () => {
+        const wrapper = shallow(<Image {...{ [propName]: 'foo' }} />)
+
+        wrapper.should.have.tagName('img')
+        wrapper.should.have.prop(propName, 'foo')
+      })
+
+      it(`passes "${propName}" to the img tag when wrapped`, () => {
+        shallow(<Image wrapped {...{ [propName]: 'foo' }} />)
+          .find('img')
+          .should.have.prop(propName, 'foo')
+      })
+    })
+  })
+
   describe('ui', () => {
     it('is true by default', () => {
       Image.defaultProps.should.have.any.keys('ui')
@@ -68,14 +87,10 @@ describe('Image', () => {
   })
 
   describe('wrapped', () => {
-    it('applies all img attribute props to the img tag', () => {
-      const props = { src: 'http://g.co', alt: 'alt text', width: 10, height: '10' }
-      const img = shallow(<Image {...props} wrapped />)
-        .find('img')
-
-      _.each(props, (val, key) => {
-        img.should.have.prop(key, val)
-      })
+    it('renders an div tag when true', () => {
+      shallow(<Image wrapped />)
+        .type()
+        .should.equal('div')
     })
   })
 })