pico icon indicating copy to clipboard operation
pico copied to clipboard

Modal component is relying too much on Javascript

Open Atmos4 opened this issue 1 year ago • 9 comments

Currently there is a strong dependency on Javascript with the modal component, especially with animations and disabling scroll. Both can be done with pure CSS instead.

  1. Animations can be done with CSS transitions

  2. Disabling scroll can be achieved with CSS :has pseudo-class.

:has is relatively new in terms of browser support, but it seems to be used in the codebase already so it's probably not a concern.

Here is a link to the demo on CodePen.

Let me know what you think. If you like the idea, I can submit a PR.

Atmos4 avatar Feb 17 '24 22:02 Atmos4

:has is relatively new in terms of browser support, but it seems to be used in the codebase already so it's probably not a concern.

Until then I had used :has as little as I could. But since ~December 2023, we can consider that we are free to use it wherever we need it. ~91.96% usage.

lucaslarroche avatar Feb 18 '24 03:02 lucaslarroche

Here is a link to the demo on CodePen.

Great demo

If you like the idea, I can submit a PR.

@Atmos4, I love it.

Let's also add button[formmethod="dialog"] to target the close button, but keep .close and [rel="prev"] to avoid breaking changes.

lucaslarroche avatar Feb 18 '24 03:02 lucaslarroche

Nice I will start working on that then!

Let's also add button[formmethod=dialog]

See my comment on the previous issue about the limits of button[formmethod] as close icon.

Atmos4 avatar Feb 18 '24 17:02 Atmos4

There is one issue with this @lucaslarroche: if I want closing transitions to display properly, I need to always have display != none and visibility: visible, which will always make the content of the modal focusable.

This can be fixed with visibility: hidden and transition-behavior: allow-discrete, but browser support is very bad (i.e. doesn't work on Firefox)

In my opinion it's still an upgrade from the current CSS behavior, and transition-behavior will gracefully degrade to no animation so I guess it's OK? What do you think?

Atmos4 avatar Feb 28 '24 16:02 Atmos4

I'm doing this and it works perfectly (except for click outside to close). Might be worth sharing it in the docs:

<dialog id="modal">
  <article>
    <header>
      <button aria-label="Close" rel="prev" onclick="modal.close()"></button>
    </header>
    <p>...</p>
  </article>
</dialog>

<button onclick="modal.showModal()">Open Modal</button>

pietz avatar Mar 27 '24 14:03 pietz

@pietz This is fine, but you will never have closing animations displaying properly... unless you always display the modal, which makes it always focusable (see my previous comment).

Atmos4 avatar Mar 28 '24 11:03 Atmos4

@Atmos4 That's true and I don't really care if it's added to the docs or not. However, given Picos philosophy and design principles, I think that my provided solution is the closest to what the average PicoCSS user expects to see: Preferring very simple HTML-only solutions that might miss a feature or two over complex or cryptic feature complete solutions. I might be wrong though :)

pietz avatar Mar 28 '24 21:03 pietz

@pietz I think we pretty much agree on most things here :) see the demo link I posted in the issue, it looks like your solution regarding JS is roughly the same as mine.

I will publish a PR about the animations and changes to the modal. I can try to update the docs too :)

Atmos4 avatar Mar 29 '24 08:03 Atmos4

FYI here is the related PR - #503

Atmos4 avatar Mar 29 '24 08:03 Atmos4