Skip to content

Conversation

tr00st
Copy link
Contributor

@tr00st tr00st commented Oct 2, 2025

As per the use case in #52 - this PR adds a fallback location to mount InPortal content when no OutPortal is present. Intended to handle continuing video playback when an OutPortal being present can't be guaranteed.

Still needs a run through handling if the Ref's .current value changes, to ensure remounting and unmounting occurs when that value is changed.

@tr00st
Copy link
Contributor Author

tr00st commented Oct 2, 2025

Just spotted the changes pushed in the mean time, will get this updated on my end


return <MyComponent componentToShow='component-a' />
})
.add('default to fallback element when not in use', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to rebase and re-add this, I've been following on from your updates, and on main this is now html.stories.tsx, written in TypeScript, and using the latest Storybook + Vite.

Copy link
Contributor Author

@tr00st tr00st Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased and fixed to work with the new storybook/typescript structure (fixes in 9b9efce)


return <div>
<div>
<InPortal node={portalNode} keepMounted>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's keepMounted doing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a leftover from an initial attempt using a different API - removed with 9b9efce

portalNode.element
);

parent = newParent;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be nice to somehow do this using mount() instead, just so we have a single implementation for this instead of separating the fallback mount & normal mount cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a look at that when working through this on one of the other API options.

Can't remember the exact details - but I vaguely recall trying to keep things slightly distinct, as with this particular setup, I'd went for a container-like mount method to ensure a the hidden div would be wrapped the portal content without needing to directly consider the fallback wrapper. There are definitely other ways to handle that though, so I'll keep it in mind.

src/index.tsx Outdated

type BaseOptions = {
attributes?: { [key: string]: string };
fallbackMountNode?: React.MutableRefObject<Node>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API is convenient and makes sense, but as you mention in #52 there are other alternatives. The challenge is making them behave reasonably - it's not clear what happens if there's two FallbackOutPortals for example.

This API has it's own quirks though - most notably because ref values can change, but this won't follow those - it'll stay rendered wherever the ref was at the last unmount call, and if the fallback node is somehow removed from the DOM and replaced, the portal content will just quietly disappear.

One option would be to give PortalNode a setAsFallback property, and using that itself as a ref, like <div ref={myNode.setAsFallback}> - in this case, the portal node will get pinged whenever the fallback is changed, and so it can track that as internal state, and migrate between fallback nodes if there's not one currently set. But maybe that's effectively equivalent to FallbackOutPortal and it would be better to consolidate the APIs... Both have the multiple-nodes issue really - but we could detect this, and error or print a warning if you ever end up in that state.

What do you think? I'm happy to explore the concept, but the API is the main challenge.

Oh the other final option would be to do something like fallback='offscreen-div', where we just support a fixed list of built-in behaviours and manage the fallback DOM entirely within this library. Much less flexible, but relatively simple, and might be worth exploring if everything else ends up too hard and we don't actually have any other use cases anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been considering APIs and how other use cases would apply over the weekend myself. Might've gone a little off the beaten track here, so apologies in advance if it's a bit rambling 😄

I guess for other use cases API-wise we'd want to consider "what's the use case?" and "what else is needed to make that use case work?"

The main alternative I'd considered is using it to simplify application logic a little if you're building a Primary/Secondary type scenario. For example, consider a video playback with a small "Preview" (Primary) and larger "Enlarged" (Secondary) view. The basic logic being - if the Enlarged view shows, then the Preview can't show.

Currently the developer needs to maintain a global state (eg: useContext, Redux, etc), to achieve this. Not the end of the world, but adds a little extra leg-work. Using a fallback method like the proposed, you could simply mount/unmount the Enlarged view, and the Primary would automatically reappear.

If you take that scenario all the way down, we might want to address:

  • Multiple levels of fallback, or a "priority" order - eg: if not even the "Primary" should be mounted, mount in an offscreen div
  • Allowing for alternative content to display in an OutPortal when something else has "priority"

(For context - the above scenario is actually what we're implementing my end. We're writing a specialist video conferencing application, and one of the issues we're having is that our Enlarged view can get shown in several different scenarios/DOM-locations, with some complex logic on how it might be hidden by other user actions. Add in occasions where neither should be displayed, and... well, hence the original issue and PR.)

Admittedly, with all the above, it's a bit more complex. Given how tight the library currently is, I'm wondering if this would make more sense as a higher level wrapper, leaving the core mount/unmount self-contained and relatively simple. The layer on that could then be a couple of components dedicated to maintaining the state and logic for multiple OutPortals, something like:

const Example = () => {
    return <ReversePortalProvider
        // One/more of these?
        render={() => <ExpensiveComponent />}
        component={ExpensiveComponent}
    >
        <ReversePortalMountPoint priority={0} hidden /> // convenience shortcut for "dump me in a hidden div?"
        <ReversePortalMountPoint priority={10} placeholder={<p>This content is currently elsewhere.</p>} />
        <ReversePortalMountPoint priority={20} />
        <ReversePortalMountPoint priority={30} />
    </ReversePortalProvider>;
};

Very much arbitrary naming, but hopefully the above makes sense. Not sure if it's a little overly complex, but at worst it should be possible to achieve that with some liberal useContext() usage to allow the ReversePortalProvider to maintain a list of active mount points enable/disable them as needed. Would again need a little care around duplicate priorities, but as you've mentioned, detecting duplicates and logging/throwing errors shouldn't be too difficult if we're maintaining a list of mounts.

Fully understand if this is a bit much in terms of scope-creep. It's obviously your choice where the bounds of the library go, but figured it's worth digging deeper.

... I do also like the fallback='offscreen-div' option - it would certainly be a lot simpler to implement, and would certainly satisfy my immediate needs.

…ration, removing outdated props from initial implementation attempts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants