GavinJoyce / ember-headlessui

https://gavinjoyce.github.io/ember-headlessui/
Other
92 stars 34 forks source link

Add modal with transition example #129

Closed roomman closed 2 years ago

roomman commented 2 years ago

This PR adds an example of a modal dialog, with nested transitions for the overlay, and modal containter.

roomman commented 2 years ago

Hello all, I would be interested in any feedback on this draft PR, which adds an example of a Modal with Transitions.

Implementing this has exposed a few, minor inconsistencies when compared with the headlessui.dev docs. For example:

On the last point, headlessui's docs offer this code example:

<TransitionRoot appear :show="isOpen" as="template">
  <span class="inline-block h-screen align-middle" aria-hidden="true">
    &#8203;
  </span>
  <TransitionChild
    as="template"
    enter="duration-300 ease-out"
    enter-from="opacity-0 scale-95"
    enter-to="opacity-100 scale-100"
    leave="duration-200 ease-in"
    leave-from="opacity-100 scale-100"
    leave-to="opacity-0 scale-95"
  >
      <div
        class="inline-block w-full max-w-md p-6 my-8 overflow-hidden text-left align-middle transition-all transform bg-white shadow-xl rounded-2xl"
      >
        {{! more code here }}
      </div>
  </TransitionChild>
</TransitionRoot>

In this example, the transition-all and transform classes are applied to the element nested within the TransitionChild and this is sufficient, and the transition works. The span element (which is used to centre the content on screen) sits above and outside of the TransitionChild.

In this PR the implementation had to be different, to get the transition to work:

<Transition @show={{this.isOpen}} as |t|>
  <t.Child
    @appear={{true}}
    @enter='ease-out duration-300'
    @enterFrom='opacity-0 scale-95'
    @enterTo='opacity-100 scale-100'
    @leave='ease-in duration-200'
    @leaveFrom='opacity-100 scale-100'
    @leaveTo='opacity-0 scale-95'
    class='transition-all transform'
  >
    <span
      class='hidden sm:inline-block sm:align-middle sm:h-screen'
      aria-hidden='true'
    >
      ​
    </span>

    <div
      class='inline-block px-4 pt-5 pb-4 overflow-hidden text-left align-bottom transition-all transform bg-white rounded-lg shadow-xl sm:my-8 sm:align-middle sm:max-w-sm sm:w-full sm:p-6'
    >
    </div>
  </t.Child>
</Transition>

Here, the transition-all and transform classes need to be added to t.Child itself, or the transition wasn't applied to the nested element. The span element has to be nested inside t.Child or the content wasn't aligned properly on the screen.

How consistent do people feel this implementation should be, when compared to the docs over at headlessui? As we don't have any docs yet, I tend to start with the headlessui examples, and then look through tests in this repo, and put the pieces together. How are others working, and is it too early to think about some documentation?

NullVoxPopuli commented 2 years ago

:wave: thanks for this lovely contribution!!

can you delete the package-lock.json?

this repo uses pnpm instead of npm -- so installing deps would be pnpm install instead of npm install.

roomman commented 2 years ago

@NullVoxPopuli done, thanks.

On a related note, now the project is using pnpm I'm experiencing hanging when I run an ember install... command.

NullVoxPopuli commented 2 years ago

On a related note, now the project is using pnpm I'm experiencing hanging when I run an ember install... command.

ah! good catch -- we need to upgrade ember-cli for pnpm support. whoops. if you want to do that, that'd be a huge help! <3