Conversation
packages/react/src/index.js
Outdated
| setData, | ||
| url, | ||
| apiKey, // destructuring to avoid pass it | ||
| placeholderComponent: CardEmpty, |
There was a problem hiding this comment.
not sure what could be the right naming for this:
- placeholderComponent
- skeletonComponent
- loadingComponent
I think *Component suffix is important to don't confuse it with a regular API parameter option
There was a problem hiding this comment.
Another structure for this sort of situation is a components prop, that receives an object where the keys are the component names in PascalCase (this is how react-select handles things).
components={{ PlaceholderComponent: Foo }}
breadadams
left a comment
breadadams
left a comment
There was a problem hiding this comment.
Looks good to me @Kikobeats, just a couple of questions:
- Regarding adding the
microlink_card_placeholderclassName to all placeholder elements. Maybe we should vary it, or add some "secondary" className to easily identify each part of the placeholder. - Should we be adding a classname to the media whilst it's in placeholder mode?
- You're not actually using the
placeholderComponentprop it seems, is that intentional?
|
Hey @Kikobeats, what do you think of this implementation? Personally after reviewing it again on GH I think I'd prefer a name such as Thanks
|
+ Add new prop for injecting custom loading component + Update stories
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
2 participants
This PR contains two things:
demo
closes #249