[✨] Built-in Portal Support

FlatMapIO commented 1 year ago

Is your feature request related to a problem?

There doesn't seem to be a good way to implement Portal functionality right now. Applications can overcome this, but it may be difficult when creating an a11y-friendly component library with complex Overlay behavior

Describe the solution you'd like

Providing <Portal /> components or other qwikized ways

Describe alternatives you've considered

No

Additional context

No response

manucorporat commented 1 year ago

Looking for a good use case for this functionality, wonder if this UI can be built easier today with Dialog or the new popover API!

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog https://developer.chrome.com/blog/introducing-popover-api/

FlatMapIO commented 1 year ago

daisyui uses the dialog element to implement Modals

Advanced component libraries like radix-ui and ark-ui implement it themselves and rely on Portal. Assuming ark-ui will add a qwik version in the future, Portal can provide a lot of help.

a11y-dialog summarizes some of the problems with the native dialog. However, "Clicking the backdrop does not close the dialog on Chrome" mentioned in it is not a problem in daisyui.

The current support for the popover element may not be widely adopted within three years.

GrandSchtroumpf commented 1 year ago

We had some discussions about using the <dialog> element on Qwik UI. For me the two issues with the <dialog> element are :

::backdrop cannot be animated.
you cannot set role="alertdialog" on the dialog element. Everything else can be easily fixed with a little bit of JS (that you would have to do anyway if you re-implement everything).

thejackshelton commented 1 year ago

@GrandSchtroumpf yeah I saw this issue in Adam Argyle's dialog post. Hopefully they add animations to ::backdrop in the future. Why is there an issue with setting a role on it?

@manucorporat If we were to have a built-in portal, I looked into Solid's implementation and it seems like it might be the easier to "convert" a similar implementation to Qwik due to the Solid having signals and JSX along with a similar "React-like" API.

I think the challenging part would be learning about other solid primitives, and it might be easier to build one from scratch in Qwik I'm not sure.

I think if this were implemented it would be a PR to Qwik core and not Qwik UI, since I can assume not everyone would want to use the Qwik UI library and some would prefer to have their own custom portal implementations.

Here's the Portal component in solid along with their vitest tests for it. https://github.com/solidjs/solid/blob/main/packages/solid/web/src/index.ts https://github.com/solidjs/solid/blob/main/packages/solid/web/test/portal.spec.tsx

jessezhang91 commented 1 year ago

Given dialog's support is ~92% (via caniuse) and popover's support being ~38%, my team didn't feel comfortable using native browser support for these portal cases.

We ended up implementing something portal-like by encapsulating the portal content in QRL<() => JSXNode> functions that we passed to portal slots and rendered them using <RenderOnce children={fn()} />. We had to hack around serialization (via noSerialize) but since we were only doing this for dialog/popovers which required user interaction first anyway, this was always going to be client-side-only. (I believe we originally had the portal slot render using <div children={...} /> but had to change it to <RenderOnce children={...} /> after upgrading from 0.16.2 to 0.19.0.)

With that said, we would love to see built-in portal support so that (1) we have confidence in the maintainability of whatever we build versus the hacky solution that may end up breaking in a subsequent version of Qwik, (2) rendering of portals may be optimized to be made more efficient/etc., and (3) potentially have portals functioning in server-side contexts as well (e.g. render a modal pre-opened as part of SSR).

joshdchang commented 1 year ago

What I've been doing that seems to work well and plays nicely with Qwik (I don't feel like I'm breaking any rules or best practices) is using context, signals, resources, and QRLs all in conjunction with each other, although I do agree that having a common use case like a modal require a deep understanding of the framework is probably something that could be improved.

In the layout, I create a context for the portal content:

import {
  Resource,
  Slot,
  createContextId,
  useContextProvider,
  useSignal,
  useResource$,
  component$,
  type QRL,
  type Signal,
  type JSXNode,
} from "@builder.io/qwik";

export const PortalContext = createContextId<Signal<QRL<JSXNode> | undefined>>("portal");

export default component$(() => {

  const portal = useSignal<QRL<JSXNode>>();
  useContextProvider(PortalContext, portal);

  const portalResource = useResource$(({ track }) => {
    track(() => portal.value);
    return portal.value?.resolve();
  });

  return (
    <>
      <Slot />
      <Resource
        value={portalResource.value}
        onResolved={(portalNode) => <>{portalNode}</>}
      />
    </>
  );
});

Then, when I wish to use the portal, I can create a component for the portal content and pass that to the portal context:

import {
  component$,
  useContext,
  $,
  useOnDocument,
} from "@builder.io/qwik";
import { PortalContext } from "../layout";

// content of the modal
export const Modal = component$(() => {

  const portal = useContext(PortalContext);

  // example of allowing `esc` to close modal
  useOnDocument(
    "keydown",
    $((event) => {
      if (event instanceof KeyboardEvent && event.key === "Escape") {
        portal.value = undefined;
      }
    })
  );

  return (
    <>
      <div>
        <button onClick$={() => (portal.value = undefined)}>
          Close modal
        </button>
      </div>
    </>
  );
});

// content of the page
export default component$(() => {

  const portal = useContext(PortalContext);

  return (
    <>
      <button
        onClick$={() => {
          portal.value = $(<Modal />);
        }}
      >
        Open modal
      </button>
    </>
  );
});

mees-van-wel commented 1 year ago

Having a native portal component, as discussed above, would be a welcome addition. Specifically, it would be beneficial to have a function akin to React's createPortal.

In my attempt to build a component that places its children into a portal, I encountered some limitations with the existing example provided by @joshdchang. I wanted a solution that would allow a to be passed to the portal.value, so I created the following:

import { component$, useContext, useVisibleTask$, Slot, useId } from "@builder.io/qwik";
import { UiContext } from "../../context/uiContext";

export const Portal = component$(() => {
  const { portalRootId } = useContext(UiContext);
  const portalContainerId = useId();

  useVisibleTask$(async ({ cleanup }) => {
    const portalContainerElement = document.querySelector(`#${portalContainerId}`) as Element;
    const portalRootElement = document.querySelector(`#${portalRootId}`) as Element;
    const portalWrapperElement = document.createElement("div");

    portalWrapperElement.innerHTML = portalContainerElement.innerHTML;
    portalContainerElement.innerHTML = "";
    portalRootElement.appendChild(portalWrapperElement);

    cleanup(() => {
      portalRootElement.removeChild(portalWrapperElement);
    });
  });

  return (
    <div id={portalContainerId} class="opacity-0 min-h-0 h-0 max-h-0 min-w-0 w-0 max-w-0 absolute">
      <Slot />
    </div>
  );
});

Though this code does work for simpler components, it fails with more complex ones, producing a Qwik error indicating that the portal content isn't connected to the (V)DOM:

[QWIK ERROR] Error: Internal assert, this is likely caused by a bug in Qwik: element must be connected to the dom

This issue underscores the need for built-in support for portals, and I hope it's something that can be addressed in future updates.

wmertens commented 1 year ago

I think people are confusing portals with popovers? A portal is for rendering elements in the current context but displaying them elsewhere. This is not possible in Qwik right now and I do wonder if it's needed.

@joshdchang's solution uses $(<Foo />) which converts that into a static QRL. This means, you can't put scope variables into it. Instead, you need to send a render function or simply a component.

Here's an example of a dialog using this principle, just missing some styling to make it a popover. You can do some more signaling to indicate when it's done, e.g. a function to call that returns a promise.

import { component$, createContextId, useContext, useContextProvider, useStore, Component } from '@builder.io/qwik';

type Ctx<P extends object = any> = {
  show: boolean
  Cmp: Component<P>
  props?: P
}
const cId = createContextId<Ctx>('modal')

const useModalProvider = () => {
  const store = useStore<Ctx>({} as Ctx)
  useContextProvider(cId, store)
  return store
}

export const Modal = component$(() => {
  const ctx = useContext(cId)
  return ctx.show ? <div>
    <div>MODAL: <ctx.Cmp {...ctx.props} /></div>
    <button onClick$={()=>ctx.show = false}>close</button>
  </div> : null
})

export const Hello = component$<{name:string}>((p)=> <>Custom component saying "hello {p.name}"</>)

export default component$(() => {
  const ctx = useModalProvider()
  return <>
    <button onClick$={()=>{
      ctx.show = true; ctx.Cmp = Hello; ctx.props = {name: "there"}
    }}>Show modal</button>
    <Modal/>
  </>
});

Playground link

wmertens commented 1 year ago

@brandonpittman I guess my example needs some more scaffolding. The idea is that you mount the Modal in your app root, so it's not part of your containers.

Did you try the playground?

GrandSchtroumpf commented 1 year ago

Wouldn't Portal break streaming? It feels like a feature that can only work on the browser, but maybe I'm mistaken.

wmertens commented 1 year ago

@GrandSchtroumpf The approach I show works fine with streaming, it doesn't matter. By putting the portal at the bottom of the root component, it's even likely that the modal will already render during SSR.

The only difference with react portals is that the Hello component would be rendered in the context of the caller, not the context of the modal. I'm not convinced it's needed.

wmertens commented 1 year ago

@brandonpittman that's because you put the Modal inside the overflow div. It's supposed to be in the outermost lowest position possible.

wmertens commented 1 year ago

@brandonpittman not sure what you mean, the Hello component and its props are passed dynamically. You could also pass a render function.

wmertens commented 1 year ago

@brandonpittman not really, anything can be done in the single Modal component but it needs to be set up for it of course.

For example, you could have an array of modals to display, and useModal could return functions to show/hide/await them. The modal component would iterate through the array and render all of them and put the backdrop behind the most recent one.

I'm guessing you'd like something like

<Portal to={modalPortal}>
  <MyCmp/>
</Portal>

?

That's not possible with Qwik, because you can't access your children while using context. Maybe something can be done with a portaling Slot component, but instead you could just pass rendered children. Remember, their context will be the modal context, not the component context.

showModal({rendered:<MyCmp>...</MyCmp>})

Here's a better playground with showModal() and inline components

wmertens commented 1 year ago

@brandonpittman actually, I can do more than I expected. Here I'm embedding the close callback.

The trick is to create the context like so:

const useModalProvider = () => {
  const ctx = {
    data: useSignal(),
  } as Ctx;
  ctx.open = $(p => {
    ctx.data.value = p;
  });
  ctx.close = $(() => {
    ctx.data.value = false;
  });
  useContextProvider(cId, ctx);
  return ctx;
};

only ctx.data is reactive, but open and close get serialized and refer to the context

wmertens commented 1 year ago

But indeed, when I try grabbing context from the components I'm rendering in the modal they break.

Actually, I haven't used React portals since I made my own dialog using the above technique before portals came out.

However, looking at the docs, I wonder if they couldn't be used for a more efficient qwikify$?

one react root rendering all qwikify$ components as portals, and then vice versa being able to embed Qwik into the React DOM.

mhevery commented 1 year ago

Do you think https://github.com/BuilderIO/qwik/pull/4928 is sufficient to close this issue?

wmertens commented 1 year ago

@mhevery I see you forego the event approach after all, but in any case the example looks great!

What are your thoughts on providing a JsxNode as well, like I show in my most recent sandbox?

What is missing from the cookbook though is a mention that the context the components will render under is the popup provider's context, so when using this technique, users have to be aware of this.

thejackshelton commented 1 year ago

I went over misko's additions to the docs with the modal cookbook, and I am struggling a bit to think about how Portals (or currently called popups) would work as a primitive in Qwik UI.

Portals are used in both modal and non-modal components. A good example would be to check the DOM in Radix's examples here. The portal content is a direct child of the body tag, outside of its parent.

https://www.radix-ui.com/primitives/docs/components/dialog https://www.radix-ui.com/primitives/docs/components/popover

The most basic use case for them is when a parent has an overflow: hidden or z-index style, but you need to visually "break out" of its container. For example, global UI such as a listbox, modal or a tooltip.

For some components in Qwik UI, we would like the consumer to choose whether their component is modal or non-modal. Such as preventing scroll and keeping the focus locked inside the popup container (modal), or being able to interact with the rest of the page, but also with portal content like a popover (non-modal).

This is consistent with other headless UI libraries. Take for example a Combobox component from Solid's Kobalte library: https://kobalte.dev/docs/core/components/combobox

Portals are used in many primitives, and each component implementation usually contains a <Portal /> component somewhere in the API, and have props such as modal or preventScroll.

Their API is something like this:

<Combobox>
  <ComboboxLabel />
  <ComboboxControl>
    <ComboboxInput />
    <ComboboxTrigger>
  </ComboboxControl>

  <ComboboxPortal> <--- portal here
    <ComboboxListbox>
      <ComboboxOption />
    </ComboboxListbox>
  </ComboboxPortal>
</Combobox>

In other frameworks, this is often solved by a dedicated API such as createPortal(). However such API's don't work well with server-side rendering, and so an alternative approach is needed.

Would the popup or popupManager here be considered as a "replacement" to portals? And if so, would it be possible to render that content outside of its parent and adjacent to the direct children of the body tag for non-modals as well?

reference: https://qwik.builder.io/docs/cookbook/modal/

Also, after reading through this GitHub issue again related to portals, wout mentioned something that I have some questions about:

What is missing from the cookbook though is a mention that the context the components will render under is the popup provider's context, so when using this technique, users have to be aware of this.

reference: https://github.com/BuilderIO/qwik/issues/4575#issuecomment-1675829239

As a library author, does this mean it would be more difficult to:

use state management with context between portal/popup content and their parents?
abstract away the popup manager to consumers?

If you guys also have any ideas for how to expose this as an API for headless would love to hear it. I started a portal implementation for Qwik UI today, and came across a decent bit of confusion regarding this. I'll be looking at this again tomorrow when I get time.

This is a long post, but if it means developers using Qwik have a better experience it's worth it. Appreciate taking the time reading this guys.

cc @mhevery @wmertens

mhevery commented 1 year ago

@mhevery I see you forego the event approach after all, but in any case the example looks great!

I think the event approach is more complicated and deserves itso own cookbook.

Aspect:

How to do popup
How to know when a popup should happen

Those are two different things. The event solves the second problem.s

mhevery commented 1 year ago

@thejackshelton https://github.com/BuilderIO/qwik/issues/4575#issuecomment-1679936609

Yes, it is a long post so let me try to answer and let me know if I miss anything

Adding content to `document`

So my idea is that in root layout.tsx you do this:

<PopupManager> <-- Developer determines the location of the portal by placing this in correct location.
  <Slot>
</PopupManager>

This means that in effect:

<HTML>
  <body> 
    <!--PopupManager-->
       <Slot/> <-- where the rest of the application goes.
       <!--Place where Popups will be inserted.-->
    <!--/PopupManager-->    
  </body>
 </HTML>

Perhaps the cookbook could be clear on that, PR to fix up the wording would be great!

For some components in Qwik UI, we would like the consumer to choose whether their component is modal or non-modal. Such as preventing scroll and keeping the focus locked inside the popup container (modal), or being able to interact with the rest of the page, but also with portal content like a popover (non-modal).

What does that mean in terms of popup manager? Does it mean you have to insert it into different location?

Would the popup or popupManager here be considered as a "replacement" to portals? And if so, would it be possible to render that content outside of its parent and adjacent to the direct children of the body tag for non-modals as well?

Correct, that is the idea. Can you suggest how we could make the cookbook more clear on this?

As a library author, does this mean it would be more difficult to:

use state management with context between portal/popup content and their parents?

abstract away the popup manager to consumers?

I was hoping that the cookbook example would make that clear.

when opening up a new modal, you do it at the location of PopupManager.
You can pass any state to the component, which will be open by the PopupManager so there should not be any issue with state managment.

wmertens commented 1 year ago

I wonder what happens to the context if you use a render function. The QRL should store the lexical state, so everything should just be fine?

E.g <Popup show={signal.value} onShow$={()=><MyComp state={ctx}} /> where ctx is context from where it's defined

mhevery commented 1 year ago

I wonder what happens to the context if you use a render function. The QRL should store the lexical state, so everything should just be fine?

E.g <Popup show={signal.value} onShow$={()=><MyComp state={ctx}} /> where ctx is context from where it's defined

I like that idea. We should explore it to see if it works...

thejackshelton commented 1 year ago

Hey everyone! You can do modal / dialog portals now following the cookbook section of the docs.

If you're looking at implementing a popover portal, something similar to a dropdown, popover itself, etc. I would check out the discussion and implementation here: https://discord.com/channels/842438759945601056/1138902270105366679/1143917159756611754

QwikDev / qwik