CaptainCodeman / svelte-headlessui

HeadlessUI components for Svelte
https://captaincodeman.github.io/svelte-headlessui/
MIT License
549 stars 25 forks source link

Differences from the canonical version #50

Closed zerosym closed 1 year ago

zerosym commented 1 year ago

I'm not sure how closely this version is meant to align with the original, but here are a few things I've noticed:

If I come across any other differences I'll try to update this list.

CaptainCodeman commented 1 year ago

Hopefully it's close, but it probably isn't an exact 1:1 match. My main focus was on exploring a different approach to cut-down on bundle size, but ideally all the behavior should be made to match.

edwinwong commented 1 year ago

Hi, just joining this conversation. I started a new branch in my fork and made some changes to on-click-outside.ts in internals so that the behavior of the various components using this (Combobox, Dialog, Listbox, Menu, Popover) matches that of the canonical Headless UI more closely, i.e. the first issue where mousedown on listbox options then releasing outside the dialog should not cause it to close. My code is here:

https://github.com/edwinwong/svelte-headlessui/blob/attempted-fixes/packages/lib/src/lib/internal/on-click-outside.ts

While my changes do work and the components now behave the same as Headless UI for holding a click and releasing somewhere else, I don't think my code is up to the standard of the rest of the library. I'm also not sure my approach to solving the problem is the best, would welcome any suggestions on how to improve it.

junwen-k commented 1 year ago

I'm not sure how closely this version is meant to align with the original, but here are a few things I've noticed:

  • Mousedown on listbox options then releasing outside the dialog should not cause it to close
  • Combobox does not allow tabbing away from its input field
  • Typing with the listbox open does not focus on matched item. (Though the canonical implementation is also lacking compared to a native select where you can type to select without needing to expand the options first).

If I come across any other differences I'll try to update this list.

Just realized that the focus trap was mentioned in this comment.

Linking the issue https://github.com/CaptainCodeman/svelte-headlessui/issues/58 for tracking purposes.

CaptainCodeman commented 1 year ago

I'm also not sure my approach to solving the problem is the best, would welcome any suggestions on how to improve it.

Sorry, I didn't see this until I'd worked on it for another issue. I ended up with something slightly different, but very similar. One key thing is to handle unsubscribing from any event listeners as per the comment in your code. I did it by saving the array of listener unsubscribers and calling them:

https://github.com/CaptainCodeman/svelte-headlessui/blob/43979203b58b62a78e92a453266e66dee96621f1/packages/lib/src/lib/internal/on-click-outside.ts