feat: add configurable portal target - #500
Conversation
|
Someone is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Avoid accessing document during render when fullscreen is closed; addresses review feedback on vercel#500.
farnabaz
left a comment
farnabaz
left a comment
There was a problem hiding this comment.
Thanks for the PR @slavco86,
I like the idea of having portal container option, but we also have similar feature for link and table modals. It make no sense to have this feature for mermaid but not for others.
Instead of mermaid.fullscreenPortalContainer we can introduce portal directly as Streamdown prop and use it in all of modal.
Are you happy to update your PR to create this prop?
|
@farnabaz , thanks for picking this up and commenting! Yes, absolutely - that makes perfect sense. Leave it with me |
|
@slavco86 one other thing, I appreciate if you sign all your commits, new and old ones. It is organization policy, all commits should be signed in order to merge a PR. (otherwise PR is blocked) https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits |
Allow built-in overlays to stay inside host CSS and stacking contexts by sharing one top-level portal target across Mermaid fullscreen, table fullscreen, and the link safety modal. Refs vercel#499
7f29b20 to
9bba54c
Compare
slavco86
changed the title
|
@farnabaz Updated as requested:
The full package build, website build, 1,144-test suite, and lint/format checks pass locally. Ready for another review when convenient. |
| prevProps.plugins === nextProps.plugins && | ||
| prevProps.className === nextProps.className && | ||
| prevProps.linkSafety === nextProps.linkSafety && | ||
| prevProps.portal === nextProps.portal && |
There was a problem hiding this comment.
@slavco86 If we pass function to portal, when other props change, component will re-render because portal will receive new instance.
I think it is good to remove this and treat portal as initializing props like other function props (urlTransform and allowElement)
Remove `portal` from the memo comparator. `PortalTarget` allows a `() => HTMLElement | null` getter, so an inline getter produced a new function identity on every parent render and defeated memoization for every consumer using that form. `portal` now behaves like the other function props (`urlTransform`, `allowElement`), which are likewise absent from the comparator. The getter form remains the recommended way to target an element that is assigned late, since it is resolved each time an overlay opens rather than at render time.
Keep the fork's package identity, deferred streaming blocks, and streamdown-context. Take unique upstream bits: configurable overlay portal (vercel#500), geistdocs 1.23.1, and the seroval bump. Skip upstream's Vercel deploy step and 2.6.0 version bump — the fork is already at 2.11.0 / remend 1.4.2.
2 participants
Summary
Adds a top-level
portalprop to configure the container used by Streamdown's built-in overlays. This keeps overlays inside a host subtree when using micro-frontends, scoped CSS, prefixed Tailwind utilities, or a custom stacking context.Changes
portal?: HTMLElement | null | (() => HTMLElement | null)toStreamdown.document.bodyas the backward-compatible fallback.portalchanges.A custom
linkSafety.renderModalremains host-controlled and therefore controls its own placement.Testing
pnpm build:packagespnpm build --filter websitepnpm test— 1,144 tests passedpnpm checkFixes #499