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(skeleton-loader): add ability to customize #1814
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'@twilio-paste/skeleton-loader': patch | ||
'@twilio-paste/core': patch | ||
--- | ||
|
||
[Skeleton-loader] Enable Compoonent to respect element customizations set on the customization provider. Component now enables setting an element name on the underlying HTML element and checks the emotion theme object to determine whether it should merge in custom styles to the ones set by the component author. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ import * as React from 'react'; | |
import {useSpring, animated} from '@twilio-paste/animation-library'; | ||
import {css, styled} from '@twilio-paste/styling-library'; | ||
import {Box, safelySpreadBoxProps} from '@twilio-paste/box'; | ||
import type {BoxElementProps} from '@twilio-paste/box'; | ||
import type {LayoutProps, BorderRadiusProps} from '@twilio-paste/style-props'; | ||
|
||
const AnimatedSkeleton = animated(Box); | ||
|
@@ -14,6 +15,7 @@ const StyledAnimatedSkeleton = styled(AnimatedSkeleton)(() => | |
|
||
export interface SkeletonLoaderProps | ||
extends React.HTMLAttributes<HTMLDivElement>, | ||
Pick<BoxElementProps, 'element'>, | ||
Omit<LayoutProps, 'verticalAlign'>, | ||
BorderRadiusProps {} | ||
|
||
|
@@ -25,6 +27,7 @@ const SkeletonLoader = React.forwardRef<HTMLDivElement, SkeletonLoaderProps>( | |
borderRadius = 'borderRadius20', | ||
borderTopLeftRadius, | ||
borderTopRightRadius, | ||
element = 'SKELETON_LOADER', | ||
display, | ||
height = 'sizeIcon20', | ||
maxHeight, | ||
|
@@ -59,6 +62,7 @@ const SkeletonLoader = React.forwardRef<HTMLDivElement, SkeletonLoaderProps>( | |
borderTopLeftRadius={borderTopLeftRadius} | ||
borderTopRightRadius={borderTopRightRadius} | ||
display={display} | ||
element={element} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does skeleton loader not have any proptypes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question. I think mainly the reason is going to be that because it just extends a bunch of Box props, validating those is really expensive. We used to do it on Box itself, and it turned out to be such a performance hit in development for engineers we removed them. Having to look up the values of these props on the theme is pretty expensive performance wise. Other reasoning is that the values are part of the design tokens theme, so there is a reference for you to get the right thing |
||
height={height} | ||
maxHeight={maxHeight} | ||
maxWidth={maxWidth} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
omg, this is smart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, I forgot to press submit on calling this out. 🤞🏼 it actually works. I think it will...