-
Notifications
You must be signed in to change notification settings - Fork 35
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
Persist input focus & related states between portals #47
Comments
Love this!
You can sponsor via GitHub if you'd like, there's a page here: https://github.com/sponsors/httptoolkit 😄
I'm not sure exactly what you mean about events triggering 'when the element is traversing', since they transfer instantly, but yes in general events can be a bit confusing: when they bubble up, they eventually bubble out of the InPortal rather than the OutPortal as you might expect. It seems that this is because of how React handles portals generally, and it's hard to override that behaviour. We've sort-of explored solutions in the past (#34) but didn't really reach a working solution yet so that's currently stalled. Would that explain what you're seeing?
Interesting, I'm not aware of this. I can see that the behaviour in the video is weird but it's hard to understand exactly what's going on here without fully knowing how this is built under the hood. If you can create a minimal demo in the storybook book here (https://github.com/httptoolkit/react-reverse-portal/tree/main/stories) that shows weird things happening with refs, that would be very helpful to investigate this further. |
Thanks for the quick reply!
Excellent! I've tossed a donation there ❤
Hmm, this could be the same issue I'm experiencing here. I was able to work around it by forcing the pop-out component to focus after the first render.
Surely! I'll create a minimal demo for my two use cases. Will reply with a link to my fork once that's sorted out 👍 |
Okay! Created a draft PR here: #48 Not actually intending to have this merged into the main repo, but feel free to take it over if you'd like. Lmk if there's any input I can help provide here😄 |
Thanks @Jaryt! I think in those examples, it's not related to the ref or the events themselves: it's simply that it seems focus state is not preserved when moving between OutPortal nodes. You can reproduce that even in simpler cases: .add("persist DOM whilst moving", () => {
const portalNode = createHtmlPortalNode();
return React.createElement(() => {
const [useOuterDiv, setDivToUse] = React.useState(false);
return (
<div>
<InPortal node={portalNode}>
<input />
</InPortal>
<button onClick={() => setDivToUse(!useOuterDiv)}>
Click to move the OutPortal
</button>
<hr />
<div>
<p>Outer OutPortal:</p>
{useOuterDiv === true && <OutPortal node={portalNode} />}
<Container>
<Container>
<Container>
<p>Inner OutPortal:</p>
{useOuterDiv === false && <OutPortal node={portalNode} />}
</Container>
</Container>
</Container>
</div>
</div>
);
});
}) The storybook example for this using a video does keep the video state, but using an input it loses the focus state. It's still the same element, but the browser seems to reset focus when it's moved across the DOM. However, there's good news! Chrome has just (like, 2 days ago) shipped a For now, I'm going to keep this open to look into that. I don't have much time to test and implement this myself right now, but PRs welcome in the meantime, or I'll get to it soon. Due to the low support for this today I think it's not super urgent, but it's definitely the way we should be moving medium-term. |
Yeah that tracks. I figured it had to do something more of the browser losing track of it, rather than it being "stateful". I think the ref issue on my end was causing a bit more confusion there, but two separate problems, one likely being on how the ref is handled on my end.
You're the expert! 😄 As for the new browser feature, that's excellent! This is the second new browser feature that would've made implementing these popout inputs a lot easier. The other being the new But alas, would have to ensure backwards compatibility anyways... maybe in 2035 browsers without these features will be considered legacy versions :P Again, I've worked around this for now, so not having immediate support isn't problematic for me. I appreciate the investigation here! |
Hi! Thanks so much for this library. Encountering the re-parent issue was super headache inducing but so grateful for the elegant solution y'all have provided here. Was sad to see there's no "buy me a beer" button anywhere :P
I'm using the package for an in-house popover cell solution. My re-parent use case is that as the user would interact with a non-focused cell within a grid, (let's say increment a numeric value) the value would be lost due to the state not being persistent. Reverse-portals solved that! And was excited to see that I was still able to portal the out portal. now you're thinking with portals
However, since the events being sent trigger when the element is traversing through the portals, it doesn't seem to be firing on the portal itself. Is there a work around for this? I'm going to try and solve this by capturing the events and forwarding them to the element later.
Note: this still persists even when I remove the secondary portal.
Another issue tangential to this race condition/edge case is that refs seem to be struggling when moving through the portal. So in my case, an anchorEl is losing track, and a dropdown menu is relocating to 0,0. This issue is probably something I can resolve on my own though, and not sure if there's even any avenues for the library to solve problems like this one.
Here's a demo of this occurring. Notice that when I click into the cell, the Portal-A cell has focus, but after being moved to Portal-B, it loses focus. You can also see the drop down flickering, and not seen here is when doesn't actually find the ref later.
firefox_0iFBPHa9kZ.mp4
Lmk if there's anything I can do to provide more context, or if you are aware of potential solutions here!
The text was updated successfully, but these errors were encountered: