[Ny komponent] Skeleton #1821
[Ny komponent] Skeleton #1821
Conversation
🦋 Changeset detectedLatest commit: 1cae71a The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
KenAJoh
changed the title
| } | ||
|
|
||
| export const Skeleton = forwardRef<HTMLDivElement, SkeletonProps>( | ||
| ({ className, variant = "text", ...rest }, ref) => { |
There was a problem hiding this comment.
Sitter og tenker på om denne koden hadde blitt enklere å lese og forholde seg til hvis man enten ikke hadde brukt destructuring i det hele tatt her, eller hadde brukt det på alle props som refereres. Det er litt "mentalt tungt" å huske på hvilke props som er igjen i rest når man leser resten av koden, spesielt når ytterligere to til fjernes i omit() under.
Bare en tanke. Det er jo ikke så mange props å forholde seg til uansett.
There was a problem hiding this comment.
Har gjort en refactor nå slik at det blir litt enklere å følge props 👍
There was a problem hiding this comment.
Så tanken er eget element som må legges i koden for å enable skeleton?
Har sett (feks. I https://blueprintjs.com/docs/#core/components/skeleton.css) at det bare er en class man legger på / kan legge på alle elementer. Er det enklere? Ser her vi kan bruke overrideableComponents sin as={Skeleton}
Litt åpent Spørsmål: er det da ekstra innsats for kunsumenter av komponent-APIet å "huske å toggle wrapper komponent når man venter på / får content"? Eller er det mulig for oss å ha skeleton "baked in" i komponentene våres? (falsy check på content eller noe "smart"?)
Tester enkel implementasjon av MUI sin skeleton-løsning
Snippet fra MUI docs
Demo