Skip to content
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

Open
Jaryt opened this issue Feb 4, 2025 · 5 comments
Open

Persist input focus & related states between portals #47

Jaryt opened this issue Feb 4, 2025 · 5 comments

Comments

@Jaryt
Copy link

Jaryt commented Feb 4, 2025

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!

@pimterry
Copy link
Member

pimterry commented Feb 5, 2025

And was excited to see that I was still able to portal the out portal. now you're thinking with portals

Love this!

Was sad to see there's no "buy me a beer" button anywhere :P

You can sponsor via GitHub if you'd like, there's a page here: https://github.com/sponsors/httptoolkit 😄

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

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?

Another issue tangential to this race condition/edge case is that refs seem to be struggling when moving through the portal.

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.

@Jaryt
Copy link
Author

Jaryt commented Feb 5, 2025

Thanks for the quick reply!

You can sponsor via GitHub if you'd like, there's a page here: https://github.com/sponsors/httptoolkit 😄

Excellent! I've tossed a donation there ❤

We've sort-of explored solutions in the past (#34)

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.

If you can create a minimal demo

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 👍

@Jaryt
Copy link
Author

Jaryt commented Feb 5, 2025

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😄

@pimterry
Copy link
Member

pimterry commented Feb 7, 2025

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 moveBefore DOM API: https://chromestatus.com/feature/5135990159835136. This explicitly avoids breaking some DOM state when moving nodes, which will fix #14 and should fix this issue too. We'll need to do some updates inside this library to detect & use that API where available, but once that's done it'll work for all users on that Chrome version. That's probably not helpful today since it's so new and not supported elsewhere but FF & Safari have expressed positive interest too, so it's likely that in future (say, a year from now) this will be widely supported by most users and the issue will disappear.

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.

@pimterry pimterry changed the title Persist events across portals? (When the event occurrs during the portal switching) Persist input focus & related states between portals Feb 7, 2025
@Jaryt
Copy link
Author

Jaryt commented Feb 7, 2025

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 can reproduce that even in simpler cases:

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 field-sizing css tag which is primarily only supported in chrome as well :(

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!

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

No branches or pull requests

2 participants