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

PosterCard-background #844

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 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
23 changes: 23 additions & 0 deletions playroom/snippets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,7 @@ const cardSnippets: Array<Snippet> = [
headline={<Tag type="promo">Headline</Tag>}
pretitle="Pretitle"
title="Title"
isInverse={true}
description="Description"
backgroundImage="https://source.unsplash.com/900x900/?landscape"
onClose={() => {}}
Expand All @@ -785,6 +786,7 @@ const cardSnippets: Array<Snippet> = [
headline={<Tag type="promo">Headline</Tag>}
pretitle="Pretitle"
title="Title"
isInverse={true}
description="Description"
backgroundVideo="https://fr-cert1-es.mytelco.io/2O4-xBJqiMlAfLkseq8RkXs_mv2ACV7Hnt20HqXxNl-mK7KLI3M2dAw"
poster="https://source.unsplash.com/900x900/?landscape"
Expand All @@ -797,6 +799,27 @@ const cardSnippets: Array<Snippet> = [
/>`,
},

{
group: 'Cards',
name: 'PosterCard background color',
code: `
<PosterCard
headline={<Tag type="promo">Headline</Tag>}
pretitle="Pretitle"
title="Title"
isInverse={true}
description="Description"
backgroundColor={colors.backgroundBrandSecondary}
poster="https://source.unsplash.com/900x900/?landscape"
onPress={() => {alert({ title: "pressed" });}}
button={
<ButtonPrimary small href="https://google.com">
Action
</ButtonPrimary>
}
/>`,
},

{
group: 'Cards',
name: 'NakedCard with Image',
Expand Down
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.
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.
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.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
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.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
44 changes: 44 additions & 0 deletions src/__stories__/poster-card-story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ type PosterCardArgs = {
withTopAction: boolean;
width: string;
height: string;
inverse: boolean;
backgroundColor?: string;
aspectRatio: '1:1' | '16:9' | '7:10' | '9:10' | 'auto';
};

Expand All @@ -58,6 +60,8 @@ export const Default: StoryComponent<PosterCardArgs> = ({
width,
height,
aspectRatio,
backgroundColor,
inverse,
}) => {
let icon;
if (asset === 'circle with icon') {
Expand Down Expand Up @@ -110,6 +114,20 @@ export const Default: StoryComponent<PosterCardArgs> = ({
poster: 'test',
};

const backgroundColorProps = {
onClose: closable ? () => {} : undefined,
actions: withTopAction
? [
{
Icon: IconLightningRegular,
onPress: () => {},
label: 'Lightning',
},
]
: undefined,
backgroundColor: 'test',
};

return (
<Stack space={32} dataAttributes={{testid: 'poster-card'}}>
<PosterCard
Expand All @@ -123,6 +141,7 @@ export const Default: StoryComponent<PosterCardArgs> = ({
width={width}
height={height}
aspectRatio={aspectRatio}
isInverse={inverse}
onPress={onPress ? () => null : undefined}
/>

Expand All @@ -138,6 +157,7 @@ export const Default: StoryComponent<PosterCardArgs> = ({
width={width}
height={height}
aspectRatio={aspectRatio}
isInverse={inverse}
onPress={onPress ? () => null : undefined}
/>

Expand All @@ -155,10 +175,28 @@ export const Default: StoryComponent<PosterCardArgs> = ({
width={width}
height={height}
aspectRatio={aspectRatio}
isInverse={inverse}
onPress={onPress ? () => null : undefined}
/>
</Box>
</ResponsiveLayout>

<Title1>NO MEDIA</Title1>
<PosterCard
{...backgroundColorProps}
icon={icon}
headline={headline ? <Tag type={headlineType}>{headline}</Tag> : undefined}
pretitle={pretitle}
title={title}
description={description}
aria-label="Poster card fallback inverse label"
width={width}
height={height}
aspectRatio={aspectRatio}
isInverse={inverse}
onPress={onPress ? () => null : undefined}
backgroundColor={backgroundColor}
/>
</Stack>
);
};
Expand All @@ -168,6 +206,7 @@ Default.args = {
asset: 'none',
headlineType: 'promo',
background: 'image',
backgroundColor: 'backgroundContainerBrand',
headline: 'Priority',
pretitle: 'Pretitle',
title: 'Title',
Expand All @@ -178,6 +217,7 @@ Default.args = {
width: 'auto',
height: 'auto',
aspectRatio: 'auto',
inverse: true,
};
Default.argTypes = {
asset: {
Expand All @@ -196,6 +236,10 @@ Default.argTypes = {
options: ['1:1', '16:9', '7:10', '9:10', 'auto'],
control: {type: 'select'},
},
backgroundColor: {
mapping: skinVars.colors,
control: {type: 'text'},
},
};

export const Group: StoryComponent = () => {
Expand Down
1 change: 0 additions & 1 deletion src/__type_tests__/card-type-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import Image from '../image';
<PosterCard title="title" trackingEvent={{name: 'do-something'}} />;
// @ts-expect-error backgroundImage and backgroundVideo can't be used together
<PosterCard title="title" backgroundImage="" backgroundVideo="" />;
// @ts-expect-error backgroundImage or backgroundVideo are mandatory
<PosterCard title="title" />;

(isTouchable: boolean) => <SnapCard title="title" href={isTouchable ? '/' : undefined} />;
Expand Down
15 changes: 11 additions & 4 deletions src/card.tsx
Copy link
Contributor

Choose a reason for hiding this comment

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

this usage is wrong
https://tinyurl.com/24fsdre5

the possibility to change text color its only allowed for custom background color

Copy link
Contributor

Choose a reason for hiding this comment

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

update: In windows works (or isInverse doesn't works xD) as expected. In Mac OS chrome this is happening
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"the possibility to change text color its only allowed for custom background color"
The text should only change the color if it receives a background color, in the case of an image, should the color be default?

"update: In windows works (or isInverse doesn't works xD) as expected. In Mac OS chrome this is happening"
I did not understand your comment. Can you explain better?

Copy link
Contributor

@yceballost yceballost Aug 9, 2023

Choose a reason for hiding this comment

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

The text should only change the color if it receives a background color, in the case of an image, should the color be default?

Exactly. When the card has an image, the gradient covers the need to make the text visible in most of the images. So we don't need to give the possibility to make color changes in the background image and video cases

I did not understand your comment. Can you explain better?

Sure! I tried the link from my first message in Windows, and the isInverse prop doesn't works in the same way as in Chrome in Mac OS. Quite weird in my opinion.. i don't know if anybody can check this in other computes with both OS.

In chrome window 10, isInverse prop always paint the text in white (with true and false value) and in chrome in Mac OS, the prop change color as I show in the image 🤔

Do I make myself clear?

Edited: Talked with edu, the reason was dark mode

Original file line number Diff line number Diff line change
Expand Up @@ -1184,14 +1184,16 @@ interface PosterCardBaseProps {
titleLinesMax?: number;
description?: string;
descriptionLinesMax?: number;
isInverse?: boolean;
backgroundColor?: string;
}

interface PosterCardWithImageProps extends PosterCardBaseProps {
backgroundImage: string;
backgroundImage?: string;
}

type PosterCardWithVideoProps = Omit<PosterCardBaseProps, 'actions' | 'onClose'> & {
backgroundVideo: VideoSource;
backgroundVideo?: VideoSource;
poster?: string;
backgroundVideoRef?: React.RefObject<VideoElement>;
};
Expand Down Expand Up @@ -1223,6 +1225,8 @@ export const PosterCard = React.forwardRef<HTMLDivElement, PosterCardProps>(
titleLinesMax,
description,
descriptionLinesMax,
isInverse = true,
backgroundColor,
...touchableProps
},
ref
Expand Down Expand Up @@ -1258,7 +1262,7 @@ export const PosterCard = React.forwardRef<HTMLDivElement, PosterCardProps>(
className={styles.boxed}
width="100%"
minHeight="100%"
isInverse
isInverse={isInverse}
background={
backgroundImage || backgroundVideo
? isExternalInverse
Expand All @@ -1275,7 +1279,10 @@ export const PosterCard = React.forwardRef<HTMLDivElement, PosterCardProps>(
>
{isTouchable && <div className={styles.touchableCardOverlayMedia} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

When the card has an image or video defined should use the touchableCardOverlayMedia style, but when the card has no media defined or a custom color defined, depending on if it's inverse or not should use touchableCardOverlayInverse / touchableCardOverlay

Screenshot 2023-08-10 at 09 02 18

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 did not understand your comment. Could you explain it better?

Copy link
Contributor

@aweell aweell Aug 11, 2023

Choose a reason for hiding this comment

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

Of course! there are three cases:

  • The card has media defined: The overlay that handle the :hover and pressed styles should use touchableCardOverlayMedia that makes the layer to have a black with 25% opacity when hovered and 35% when pressed, that works in most images since we don't know how dark/light they are.
  • The card has no media defined and is inverse={false}: The overlay that handle the :hover and pressed styles should use touchableCardOverlay. Since the background will be white (the default) or a light color, the styles for the overlay are more subtle.
  • The card has no media defined and is inverse={true}: The overlay that handle the :hover and pressed styles should use touchableCardOverlayInverse. Since the background will be backgroundContainerBrand (the default), backgroundContainerBrandOverInverse if it's inside an inverse context, or a dark color, the styles for the overlay should be a little bit stronger in order to be visible.

The same applies to the top actions container (image posted in the previous comment), depending of the case the container will have different default an interaction states.

  • The card has media defined: The actions container by default is inverse at 70%, and we change the opacity for the :hover (90%) and pressed (100%) states. The icon is black.
  • The card has no media defined and is inverse={false}: The actions container by default is transparent and uses the tokens backgroundContainerHover & backgroundContainerPressed for the interaction states. The icon uses neutralHigh.
  • The card has no media defined and is inverse={true}: The actions container by default is transparent and uses the tokens backgroundContainerBrandHover & backgroundContainerBrandPressed for the interaction states. The icon uses inverse.

I hope the explanation is more clear now 😃


<div className={styles.displayCardContainer}>
<div
className={styles.displayCardContainer}
style={backgroundColor ? {background: backgroundColor} : undefined}
>
<ThemeVariant isInverse={isExternalInverse}>
<div className={styles.displayCardBackground}>
{backgroundVideo ? video : backgroundImage ? image : undefined}
Expand Down
Loading