search

GavinJoyce / ember-headlessui

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

Add Combobox component #165

Closed balinterdi closed 2 years ago

balinterdi commented 2 years ago

Co-authored-by: David McNamara david.mcnamara@phorest.com

The original implementation by done by David but the file structure of the add-on has been drastically changed so the feasible way was to copy his work from the previous place.

Also, Ember seems to have gotten way stricter about throwing the "You attempted to update activeOptionIndex but it'd already been used previously in the same computation" errors so a lot of the pieces needed to be refactored to make tests pass.

dmcnamara-eng commented 2 years ago

Somethings not quite right here.

I should be able to hit enter to select the first item in the list.

Compare this implementation compared to react.

https://user-images.githubusercontent.com/70960245/193063249-1caeb917-cb50-4166-aea5-f3428662218e.mov

https://user-images.githubusercontent.com/70960245/193063524-43a1f502-c543-464f-bd09-d3e0932292ad.mov

balinterdi commented 2 years ago

I should be able to hit enter to select the first item in the list.

Indeed, that's not covered by tests, I'll take a look.

balinterdi commented 2 years ago

@dmcnamara-eng I think I fixed it now!

rreckonerr commented 2 years ago

Hi, all! Thanks for implementing the combobox! I'd really like to try it out but I still didn't figure out how to install ember-headlessui from this branch. Got any tips?

balinterdi commented 2 years ago

@rreckonerr The challenge with using a branch (custom SHA) of a forked monorepo is that the top-level folder of the github repository is not the same as the package's folder. Here, the package lives under /ember-headlessui in the repo.

The solution I've found is to use a service called gitpkg that allows to "link" to a subfolder in the dependency string.

Here is what we use as the dependency for ember-headlessui in our package.json:

https://gitpkg.now.sh/phorest/ember-headlessui/ember-headlessui?028782f7e65d8942c4d92d0e5df42f97c3b96fc0

That SHA at the end might change but you get the point. (You can even create a fork in your repo to have more control over the version you use).

rreckonerr commented 2 years ago

@balinterdi Thanks for the quick response! Do you also use pnpm for that? I think I hit this issue when I tried to install it from the gitpkg the last time

balinterdi commented 2 years ago

No, in this project we use yarn although I don't see why it should matter – I think it's just standard package.json syntax.

NullVoxPopuli commented 2 years ago

This project uses pnpm.

Yarn is riddled with hoisting bugs in monorepos.

The best way to test this package, pre-release, is to clone locally and link (npm/yarn/pnpm).

It went to a monorepo format for incrementally moving to a v2 addon, which presently requires a monorepo (until bottled-ember work progresses). I got hung up on a bunch of weird things working in auno-import via node shims that shouldn't be in the browser ever, and i stepped away for a while:( Apologies for the inprogress migration

balinterdi commented 2 years ago

@NullVoxPopuli Thank you for explaining.

I meant that the host project that uses this fork of ember-headlessui uses yarn.

I first tried to yarn link it locally to test but it didn't seem to work at all – the ember-headlessui component files didn't make it into the build of the host app. After trying in vain for half a day, I found gitpkg and it worked on the first try.

dmcnamara-eng commented 2 years ago

Thanks for updating this @balinterdi

Seems to be API complete at this point and robust enough to merge. 👍 ⭐ 💥