Next.js: Swap out ally.js for focus-trap-react

[ricokahler]

Feature request

Is your feature request related to a problem? Please describe.

In the discussions, a few of us have noticed some issues with the ally.js package that's imported in @next/react-dev-overlay.

I'm wondering if you'd be willing to swap our ally.js for focus-trap-react. It appears the only place where ally.js is used is in the @next/react-dev-overlay and in that package, ally.js is only being used to trap focus.

Describe the solution you'd like

…It seems like focus-trap-react is more _focused_ 😅

Its bundle size is much smaller too vs ally.js

Describe alternatives you've considered

There may be other libs I haven't considered but this one seems the most popular.

Additional context

If you're accepting, I'd be willing to throw this together — at least to experiment.

[ricokahler]

Another idea could be just removing the focus trap feature all together. When the development overlay modal opens, we could just focus it without trapping.

[unbelivr]

I see that ally.js is not only being used to trap focus.

There next.js disables any type of user interaction, so focus-trap-react seems not to be correct replacement.
const handle1 = allyDisable({ filter: overlay })

[Timer]

Focus trapping is necessary for the DX of the overlay. Can you share more about this error? What specifically is going wrong?

[ricokahler]

@Timer ,

@chrisvariety had a repro for this issue here: https://github.com/vercel/next.js/discussions/14056#discussioncomment-28461

It has something to do with platform.js in ally.js.

—-

I think this issue should’ve been regarding that platform.js issue vs replacing ally.js with focus-trap-react (my bad!). I just saw that the bundle size for ally.js was relatively large and I thought I throw this idea out there for
potentially faster dev times but I’m running into an XY problem situation.