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

Converts Card to typescript #1160

Merged
merged 2 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
111 changes: 0 additions & 111 deletions src/Card/Card.jsx

This file was deleted.

98 changes: 98 additions & 0 deletions src/Card/Card.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import React, {
createElement,
type DetailedHTMLProps,
type HTMLAttributes,
type ReactNode,
} from 'react';
import classNames from 'classnames';

import { LoadingSkeleton } from 'src/LoadingSkeleton';

import './Card.scss';

export const CardSizes = {
EXTRA_SMALL: 'xs',
SMALL: 'sm',
MEDIUM: 'md',
LARGE: 'lg',
} as const;
Comment on lines +13 to +18
Copy link
Collaborator Author

@kyleshike kyleshike Feb 26, 2024

Choose a reason for hiding this comment

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

I've kept this here for backward compatibility's sake, but it's not really necessary in Typescript. As we get further along with adoption, we can explore just removing these types of objects altogether.


type ElementProps = DetailedHTMLProps<HTMLAttributes<HTMLElement>, HTMLElement>;
Copy link
Contributor

@AlexAlt AlexAlt Feb 26, 2024

Choose a reason for hiding this comment

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

what is happening here with this syntax? like, what does it mean or how is it defining this type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea so this is using what are called generics, which we won't be getting to for a few weeks in the course.

In essence, this type evaluates to the set of all possible props that can be passed to an HTML element (as far as React is concerned). So really, it's the firehose of HTML props, including all even listeners, all aria attributes, etc, etc, etc.

The only reason that I'm using it here is because the card component is spreading all its props into the element that it renders. This is an anti-pattern IMHO, but I didn't want to change the API of this component in this work, just convert it to TS:

return createElement(
elementType,
{
...props,


type CardProps = {
children?: ReactNode;
className?: string;
divided?: boolean;
elementType?: string;
helperText?: ReactNode;
Copy link
Collaborator

Choose a reason for hiding this comment

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

helperText was previously string, tho I do recall people wanting that bit of flexibility by accepting node here as well as the other text based props in order to work around some layout issues, but I think that's for another discussion about the Card so this works fine.

isLoading?: boolean;
loadingSkeleton?: ReactNode;
loadingSkeletonParagraphCount?: number;
noPadding?: boolean;
size: 'xs' | 'sm' | 'md' | 'lg';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead, we can explicitly state that size can only one of these four values

subTitle?: ReactNode;
title?: ReactNode;
} & ElementProps;

const Card = ({
children,
className,
divided = false,
elementType = 'section',
helperText,
isLoading = false,
loadingSkeleton,
loadingSkeletonParagraphCount = 1,
noPadding = false,
size,
subTitle,
title,
...props
}: CardProps) => {
const defaultLoadingSkeleton = (
<>
<LoadingSkeleton height={24} width="33%" />
<br />
{Array(loadingSkeletonParagraphCount).fill(0).map((_, i) => (
// eslint-disable-next-line react/no-array-index-key
<div className="Card__loading-skeleton-paragraphs" key={`skeleton-paragraph-${i}`}>
<LoadingSkeleton count={3} />
</div>
))}
</>
);

return createElement(
elementType,
{
...props,
className: classNames(
'Card',
{ [`Card--${size}`]: size },
className,
{
'Card--divided': divided,
'Card--no-padding': noPadding,
},
),
},
<>
{isLoading ? (loadingSkeleton || defaultLoadingSkeleton) : (
<>
{title && (
<div className="Card__header">
<h2 className="Card__title">{title}</h2>
{helperText && <span className="Card__helper-text">{helperText}</span>}
</div>
)}

{divided && <hr className="Card__divider" /> }
{subTitle && <h3 className="Card__subtitle">{subTitle}</h3> }
{children}
</>
)}
</>,
);
};

export default Card;
File renamed without changes.
Loading