Skip to content

feat: refactor Tile removing ProductTile component #548

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.

Sign up for GitHub

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

Merged
merged 26 commits into from
Jun 4, 2019
Merged

feat: refactor Tile removing ProductTile component #548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
62182fc
product tile and content complete
May 24, 2019
bf5eff9
media tile complete
May 24, 2019
9e0bf80
updated snapshots
May 24, 2019
503f72f
added title prop to product tile content
May 24, 2019
3221d62
added tests to check for className
May 25, 2019
204e24f
added title prop to productTileContent tests
May 28, 2019
029390a
remove title specific prop from product tile content
May 29, 2019
d6ffac6
edited image for product tile on docs site
May 29, 2019
346ed81
removed default props for productTile boolean
May 30, 2019
ba69c20
product tile component examples only consume tile
May 31, 2019
310253c
passed productTile prop by cloning children
May 31, 2019
040ab92
added description for background image on media tile
May 31, 2019
4de6ab0
added conditional to clone to account for tile actions
May 31, 2019
68fa459
updated tile tests and deleted productTile.test
May 31, 2019
4045b43
product tile media tests complete with old file deleted
May 31, 2019
0a3e225
deleted product tile content tests
May 31, 2019
91af473
product tile snapshot deleted
May 31, 2019
2818cd8
deleted eslint disable
May 31, 2019
80c1dfd
deleted unneeded files and references to product tile
May 31, 2019
c79604f
media with actions example added and tileContent heading class updated
May 31, 2019
eead6c0
Merge branch 'master' into 6475refactorTile
Jun 3, 2019
a5199f4
updated example page with photo and no breaks
Jun 3, 2019
391b979
updated tests with dummy url and took away isButton
Jun 3, 2019
eae649b
background image on tile
Jun 3, 2019
1a33e6f
deleted tile playground and fixed tests to background image change
Jun 3, 2019
da92e76
took out disbled example and role=button
Jun 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 0 additions & 38 deletions src/Tile/ProductTile.js
View file
Open in desktop

This file was deleted.

47 changes: 0 additions & 47 deletions src/Tile/ProductTile.test.js
View file
Open in desktop

This file was deleted.

38 changes: 0 additions & 38 deletions src/Tile/ProductTileContent.test.js
View file
Open in desktop

This file was deleted.

26 changes: 0 additions & 26 deletions src/Tile/ProductTileMedia.test.js
View file
Open in desktop

This file was deleted.

85 changes: 36 additions & 49 deletions src/Tile/Tile.Component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path from 'path';
import React from 'react';
import { Button, Identifier, Image, Menu, Popover, ProductTile, Tile, TileGrid } from '../';
import { ComponentPage, Example, Playground, Separator } from '../_playground';
import { Button, Identifier, Image, Menu, Popover, Tile, TileGrid } from '../';
import { ComponentPage, Example } from '../_playground';

export const TileComponent = () => {
return (
Expand Down Expand Up @@ -30,7 +30,7 @@ export const TileComponent = () => {
<Tile.Content title='Tile Title' />
</Tile>
<br />
<Tile role='button'>
<Tile>
<Tile.Media>
<Image photo='https://placeimg.com/400/400/nature' size='l'
type='circle' />
Expand All @@ -40,7 +40,7 @@ export const TileComponent = () => {
</Tile.Content>
</Tile>
<br />
<Tile role='button'>
<Tile>
<Tile.Media>
<Identifier color={3} glyph='home'
size='m' />
Expand Down Expand Up @@ -75,24 +75,38 @@ export const TileComponent = () => {
</Example>

<Example
title='Product Tile'>
<div>
<ProductTile role='button'>
<ProductTile.Media image='https://techne.yaas.io/images/product-thumbnail-wide.png' />
<ProductTile.Content title='Tile Title'>
<p>Tile Description</p>
</ProductTile.Content>
</ProductTile>

<br />
title='Media with Actions Tile'>
<Tile>
<Tile.Media>
<Image photo='https://placeimg.com/400/400/nature' size='l'
type='circle' />
</Tile.Media>
<Tile.Content title='Tile Title' />
<Tile.Actions>
<Popover
body={
<Menu>
<Menu.List>
<Menu.Item url='/'>Option 1</Menu.Item>
<Menu.Item url='/'>Option 2</Menu.Item>
<Menu.Item url='/'>Option 3</Menu.Item>
<Menu.Item url='/'>Option 4</Menu.Item>
</Menu.List>
</Menu>
}
control={<Button glyph='vertical-grip' option='light' />}
placement='bottom-end' />
</Tile.Actions>
</Tile>
</Example>

<ProductTile disabled>
<ProductTile.Media image='https://techne.yaas.io/images/product-thumbnail-wide.png' />
<ProductTile.Content title='Tile Title'>
<p>Tile Description</p>
</ProductTile.Content>
</ProductTile>
</div>
<Example
title='Product Tile'>
<Tile backgroundImage='https://placeimg.com/1600/400/nature' productTile>
<Tile.Content title='Tile Title'>
<p>Tile Description</p>
</Tile.Content>
</Tile>
</Example>

<Example
Expand All @@ -118,7 +132,7 @@ export const TileComponent = () => {
<p>Tile Description</p>
</Tile.Content>
</Tile>
<Tile role='button'>
<Tile>
<Tile.Media>
<Identifier color={3} glyph='home'
size='l' />
Expand All @@ -137,33 +151,6 @@ export const TileComponent = () => {
</Tile>
</TileGrid>
</Example>

<Separator />

<Playground
component='tile'
schema={[
{
attribute: 'type',
typeOfAttribute: 'component',
'enum': ['simple', 'media', 'product']
},
{
attribute: 'title',
typeOfAttribute: 'string'
},
{
attribute: 'children',
typeOfAttribute: 'string'
}
]}>
<Tile>
<Tile.Content title='Tile Title'>
<p>Tile Description</p>
</Tile.Content>
</Tile>
</Playground>

</ComponentPage>
);
};
24 changes: 22 additions & 2 deletions src/Tile/Tile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@ const Tile = props => {
columnSpan,
colorAccent,
backgroundColor,
backgroundImage,
children,
className,
productTile,
...rest
} = props;

const tileClasses = classnames(
'fd-tile',
{
'fd-tile': !productTile,
'fd-product-tile': productTile,
'is-disabled': disabled,
[`fd-has-grid-row-span-${rowSpan}`]: !!rowSpan,
[`fd-has-grid-column-span-${columnSpan}`]: !!columnSpan,
Expand All @@ -34,7 +37,20 @@ const Tile = props => {
<div
{...rest}
className={tileClasses}>
{children}
{productTile &&
<div className='fd-product-tile__media' style={{ backgroundImage: 'url(' + backgroundImage + ')' }} />
}
Copy link
Contributor Author

@hertweckhr1 hertweckhr1 Jun 3, 2019

Choose a reason for hiding this comment

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

I created a conditional for product tile rather than if the background image exists bc product tiles should have background images with them always. This will eliminate the issue Jenna mentioned with background image rendering undefined for tiles that are not product.

Copy link
Contributor

@greg-a-smith greg-a-smith Jun 3, 2019

Choose a reason for hiding this comment

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

Well, technically it won't. There are really two props needed for a product tile: productTile and backgroundImage so it's possible the consumer passes one, but not the other. That said, this has become more of a pattern-based component so I think this is a good change. We could make productTile a shape prop with a required backgroundImage, but that might be overkill here. As we've said in this PR, the consumer just wants to know which props to pass and the example page will help them with that.

{React.Children.toArray(children).map(child => {
const isAction = child.type && child.type.displayName === 'Tile.Actions';

if (isAction) {
return child;
}

return React.cloneElement(child, {
productTile: productTile
});
})}
</div>
);
};
Expand All @@ -43,17 +59,21 @@ Tile.displayName = 'Tile';

Tile.propTypes = {
backgroundColor: PropTypes.number,
backgroundImage: PropTypes.string,
className: PropTypes.string,
colorAccent: PropTypes.number,
columnSpan: CustomPropTypes.range(1, 6),
disabled: PropTypes.bool,
productTile: PropTypes.bool,
Copy link
Contributor

@greg-a-smith greg-a-smith May 29, 2019

Choose a reason for hiding this comment

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

I think this component should clone children and pass the top-level productTile prop to each child so the prop only needs to be added at the top-level component. (see related comments)

rowSpan: PropTypes.number
};

Tile.propDescriptions = {
backgroundColor: 'Sets a background color class.',
backgroundImage: 'URL of the background image for product tile.',
colorAccent: 'Sets a background color accent class. Options include numbers from 1 to 9.',
columnSpan: 'Number of columns the tile covers.',
productTile: 'Set to **true** to mark component as a product tile.',
rowSpan: 'Number of rows the tile covers.'
};

Expand Down
Loading