Skip to content

Conversation

@revolunet
Copy link
Collaborator

add Card component

Julien Bouquillon added 3 commits December 16, 2022 09:50
Signed-off-by: Julien Bouquillon <julien.bouquillon@sg.social.gouv.fr>
@revolunet revolunet marked this pull request as ready for review December 21, 2022 11:07
Signed-off-by: Julien Bouquillon <julien.bouquillon@sg.social.gouv.fr>
Copy link
Collaborator

@garronej garronej left a comment

Choose a reason for hiding this comment

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

Thank you <3

src/Card.tsx Outdated
horizontal?: boolean;
size?: "small" | "medium" | "large"; // only affect the text
enlargeLink?: boolean; // make the whole card clickable
iconId?: FrIconClassName | RiIconClassName; // only needed when enlargeMode=true
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is enlargeMode? Wouldn't it make sence to express this in the type def?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

enlargeMode makes the whole card clickable; what do you mean by express this in the type def? ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

my bad, its not enlargeMode but enlargeLink

Copy link
Collaborator

Choose a reason for hiding this comment

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

enlargeMode makes the whole card clickable; what do you mean by express this in the type def? ?

type EnlargedLink = {
    enlargeLink: true;
    iconId?: FrIconClassName | RiIconClassName;
};

type NotEnlargedLink = {
    enlargeLink?: false;
    iconId?: never;
}

type Props = .... & (EnlargedLink | NotEnlargedLink);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yay makes sense thanks

Julien Bouquillon and others added 7 commits December 21, 2022 12:57
Co-authored-by: Joseph Garrone <joseph.garrone.gj@gmail.com>
Signed-off-by: Julien Bouquillon <contact@revolunet.com>
Co-authored-by: Joseph Garrone <joseph.garrone.gj@gmail.com>
Signed-off-by: Julien Bouquillon <contact@revolunet.com>
Co-authored-by: Joseph Garrone <joseph.garrone.gj@gmail.com>
Signed-off-by: Julien Bouquillon <contact@revolunet.com>
Signed-off-by: Julien Bouquillon <julien.bouquillon@sg.social.gouv.fr>
@garronej garronej merged commit 4f5caae into main Dec 21, 2022
@garronej garronej deleted the card branch December 21, 2022 13:12
@garronej
Copy link
Collaborator

Thank you for the adjustments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants