-
Notifications
You must be signed in to change notification settings - Fork 161
Adds ability to override components from consuming app #30
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
Conversation
src/components/asset.tsx
Outdated
src={type === "figma" ? value.properties.source[0][0] : display_source} | ||
src={ | ||
type === "figma" ? value.properties.source[0][0] : display_source | ||
} |
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.
Husky was preventing commits without this prettier change.
properties: { | ||
source: [string[]]; | ||
}; | ||
} |
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.
Adding the tweet interface here avoids the need to add it to the consuming app.
src/types.ts
Outdated
| CollectionValueType | ||
| TweetType; | ||
|
||
export type BlockValueTypeKeys = Pick<BlockValueType, "type">["type"]; |
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.
This creates a type of all the "type" keys in BlockValueType
so that we can properly type the customComponent prop.
Oops. @transitive-bullshit it looks like you've already started a version of this in #13. I'm not sure there's a ton of a benefit in a provider/consumer approach vs what I've suggested here, but I could be missing a use case you have. |
Thanks for this PR! We plan to move this forward in the coming week, when we have some time to spend on react-notion. Are there any downsides compared to the Context approach? Would love to hear @transitive-bullshit thoughts on this. |
Co-Authored-By: Tobias Lins <[email protected]>
- Useful for wrapping original components (E.g. lightbox or custom links)
- Rename customComponents to customBlockComponents
I think we can merge this now. @tobiaslins It's now possible to overwrite decorator components as well. So full flexibility with this change 🙂 Here is an example that demos both decorator components as well as blocks. It adds <NotionRenderer
blockMap={blockMap}
fullPage
hideHeader
customDecoratorComponents={{
a: ({ decoratorValue, children }) => (
<a href={decoratorValue} target="_blank" rel="noopener noreferrer">
{children}
</a>
)
}}
customBlockComponents={{
page: ({ blockValue, renderComponent }) => (
<Link href={`/${blockValue.id}`}>{renderComponent()}</Link>
)
}}
/> |
I am in the process of exporting our blog from Medium and wanted to make sure elements of our posts can be rendered by this library.
In doing so, we needed to render a tweet, but by adding it to this library it would include extra dependencies that aren't always needed. I'd like to propose adding a
customComponents
prop that takes mapping of block types and components. The components are called with theblockValue
prop.This way,
react-notion
can provide a base set of components, but everything can be overridden.Let me know what you think!
Here's what the code looks like right now in the consuming app (using Next.js):
And the component: