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

feat(MediaCard): add top icon support #824

Merged
merged 12 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 2 additions & 0 deletions playroom/snippets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,7 @@ const cardSnippets: Array<Snippet> = [
title="Title"
subtitle="Subtitle"
description="Description"
asset={<Avatar size={40} src="https://source.unsplash.com/600x600/?face" />}
extra={<Placeholder />}
button={
<ButtonPrimary small onPress={() => {}}>
Expand All @@ -635,6 +636,7 @@ const cardSnippets: Array<Snippet> = [
title="Title"
subtitle="Subtitle"
description="Description"
asset={<Avatar size={40} src="https://source.unsplash.com/600x600/?face" />}
extra={<Placeholder />}
button={
<ButtonPrimary small onPress={() => {}}>
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happened here?
Please review that extra bottom padding

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
22 changes: 22 additions & 0 deletions src/__stories__/media-card-story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ import {
Image,
Tag,
IconMobileDeviceRegular,
Circle,
skinVars,
} from '..';
import ResponsiveLayout from '../responsive-layout';
import {Placeholder} from '../placeholder';
import tennisImg from './images/tennis.jpg';
import confettiVideo from './videos/confetti.mp4';
import avatarImg from './images/avatar.jpg';

import type {TagType} from '..';

Expand All @@ -26,6 +29,7 @@ const VIDEO_SRC = confettiVideo;
const IMAGE_SRC = tennisImg;

type Args = {
asset: 'icon' | 'circle + icon' | 'image' | 'circle + image';
media: 'image' | 'video';
headlineType: TagType;
headline: string;
Expand All @@ -51,7 +55,19 @@ export const Default: StoryComponent<Args> = ({
closable,
withTopAction,
media,
asset,
}) => {
let icon;
if (asset === 'circle + icon') {
icon = (
<Circle size={40} backgroundColor={skinVars.colors.brandLow}>
<IconMobileDeviceRegular color={skinVars.colors.brand} />
</Circle>
);
} else if (asset === 'circle + image') {
icon = <Circle size={40} backgroundImage={avatarImg} />;
}

const button = actions.includes('button') ? (
<ButtonPrimary small href="https://google.com">
Action
Expand Down Expand Up @@ -79,6 +95,7 @@ export const Default: StoryComponent<Args> = ({
title={title}
subtitle={subtitle}
description={description}
asset={icon}
media={
media === 'video' ? (
<Video src={VIDEO_SRC} aspectRatio="16:9" dataAttributes={{qsysid: 'video'}} />
Expand Down Expand Up @@ -108,6 +125,7 @@ export const Default: StoryComponent<Args> = ({

Default.storyName = 'Media card';
Default.args = {
asset: 'icon',
media: 'image',
headlineType: 'promo',
headline: 'Priority',
Expand All @@ -121,6 +139,10 @@ Default.args = {
withTopAction: false,
};
Default.argTypes = {
asset: {
options: ['circle + icon', 'circle + image', 'none'],
control: {type: 'select'},
},
media: {
options: ['image', 'video'],
control: {type: 'select'},
Expand Down
16 changes: 16 additions & 0 deletions src/card.css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,22 @@ export const mediaCardContent = style([
},
]);

export const mediaCardAsset = style([
{
position: 'absolute',
zIndex: 1,
},
{
'@media': {
[mq.desktopOrBigger]: {
paddingLeft: 24,
paddingTop: 24,
paddingBottom: 32,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@aweell, it's ok to have different paddings in desktop? I don't see this in the specs.

If these are needed, we should move them to the Box element instead of creating a new media query here @thiagoaraujo-tlf:

<Box paddingLeft={{mobile: 16, desktop: 24}} paddingTop={{mobile: 16, desktop: 24}}>

Also, I think paddingRight and paddingBottom are not needed, just left and top

Copy link
Contributor

@aweell aweell Jul 26, 2023

Choose a reason for hiding this comment

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

Yes, the asset has a spacing left and top of 16 in mobile and 24 in desktop.

Mobile:
Screenshot 2023-07-26 at 17 02 43

Desktop:
Screenshot 2023-07-26 at 17 02 46

This is intended to align the asset to the content since the padding of the content increases in desktop.

The other changes, it seems they're related the content padding? They are already working in the current component.

Mobile:
Screenshot 2023-07-26 at 17 06 17

Desktop:
Screenshot 2023-07-26 at 17 05 19

Copy link
Contributor

Choose a reason for hiding this comment

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

I just added icon padding for when it's on desktop @atabel

},
},
},
]);

export const nakedCardContent = style([
sprinkles({
display: 'flex',
Expand Down
9 changes: 9 additions & 0 deletions src/card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ interface MediaCardBaseProps {
subtitle?: string;
subtitleLinesMax?: number;
description?: string;
asset?: React.ReactElement;
descriptionLinesMax?: number;
extra?: React.ReactNode;
actions?: Array<CardAction>;
Expand Down Expand Up @@ -469,6 +470,7 @@ export const MediaCard = React.forwardRef<HTMLDivElement, MediaCardProps>(
title,
titleLinesMax,
description,
asset,
descriptionLinesMax,
extra,
actions,
Expand Down Expand Up @@ -516,6 +518,13 @@ export const MediaCard = React.forwardRef<HTMLDivElement, MediaCardProps>(
buttonLink={buttonLink}
/>
</div>
{asset ? (
<Box className={styles.mediaCardAsset} paddingX={16} paddingY={16}>
{asset}
</Box>
) : (
<Box paddingBottom={actions?.length || onClose ? 64 : 0} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not needed. It's needed in other cards like DisplayMediaCard because the asset is not absolutely positioned like here.

)}
</div>
</BaseTouchable>
</Boxed>
Expand Down
Loading