search

GavinJoyce / ember-headlessui

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

Add menu with popper example #95

Open NullVoxPopuli opened 3 years ago

NullVoxPopuli commented 3 years ago

This is a breaking change, so if ya'll don't want to go along with this, that's fine by me.

the breaking change is that any consuming project must also be using ember-auto-import@v2

The demo is the same as the "Basic" Menu demo, and is only has noticeable differences on super small screens where popper starts to take control of positioning as layout constraints come in to play

GavinJoyce commented 3 years ago

the breaking change is that any consuming project must also be using ember-auto-import@v2

I think it's fine for us to make breaking changes like this 👍

FYI, it looks like there are some failing tests in CI

NullVoxPopuli commented 3 years ago

excellent! because ember-auto-import@v2 is a requirement for ember 4 :D

NullVoxPopuli commented 3 years ago

I'm not sure how any of these were passing 🤔 image

I assume most of this was copied from either the React or Vue repo?

NullVoxPopuli commented 3 years ago

It seems back when we were on webpack 4 (ember-auto-import@v1, a bunch of implicit non-browser behaviors were getting polyfilled)

alexlafroscia commented 3 years ago

I'm totally fine with making a breaking change and upgrading to ember-auto-import@2, but could that work be split out into a separate PR? I think that would make it easier to review the code and understand the affect.

In this PR at least, it seems that ember-auto-import is only a development dependency, so I'm not sure that would be a breaking change?

GavinJoyce commented 3 years ago

FYI, I've updated ember-auto-import here: https://github.com/GavinJoyce/ember-headlessui/pull/111