-
-
Notifications
You must be signed in to change notification settings - Fork 622
fix: support direct image hyperlinks #640
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
base: master
Are you sure you want to change the base?
fix: support direct image hyperlinks #640
Conversation
…ng in AssetWrapper
…nk logic and improving null checks
…tion logic for better clarity and validation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
{value?.properties?.caption && ( | ||
<figcaption className='notion-asset-caption'> | ||
<Text value={value.properties.caption} block={block} /> | ||
</figcaption> |
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.
The caption is now always displayed, even when it's being used as a link URL. This appears to contradict the PR description which mentions "exposing the link as a caption feels intrusive and affects the user experience."
Consider restoring the condition to hide captions when they're used as links:
{value?.properties?.caption && (!urlInfo || imageHyperlink) && (
<figcaption className='notion-asset-caption'>
<Text value={value.properties.caption} block={block} />
</figcaption>
)}
This would hide the caption when it's being used as a link URL (unless imageHyperlink
is also present), which aligns with the stated goal of the PR.
{value?.properties?.caption && ( | |
<figcaption className='notion-asset-caption'> | |
<Text value={value.properties.caption} block={block} /> | |
</figcaption> | |
{value?.properties?.caption && (!urlInfo || imageHyperlink) && ( | |
<figcaption className='notion-asset-caption'> | |
<Text value={value.properties.caption} block={block} /> | |
</figcaption> | |
)} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
format: { | ||
image_hyperlink: string | ||
} & BaseContentBlock['format'] |
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.
The image_hyperlink
property should be marked as optional since it won't be present on all image blocks. Consider updating the type definition to:
format?: {
image_hyperlink?: string
} & BaseContentBlock['format']
This ensures type safety when working with image blocks that don't contain hyperlinks.
format: { | |
image_hyperlink: string | |
} & BaseContentBlock['format'] | |
format?: { | |
image_hyperlink?: string | |
} & BaseContentBlock['format'] | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
fix #637
Description
adds native support for hyperlinks applied directly to image blocks. While Notion allows hyperlinks via image captions, exposing the link as a caption feels intrusive and affects the user experience.
To address this, we modify the asset-wrapper logic to properly detect and wrap images in anchor tags when a valid image_hyperlink is present.
Solution
getURLInfo()
mapPageUrl
.Notion Test Page ID
2061b06afbf1807b8e35d35e7a7fd146