search

GavinJoyce / ember-headlessui

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

Remove render modifiers #154

Closed roomman closed 2 years ago

roomman commented 2 years ago

This PR removes @ember/render-modifiers, in line with the discussion here.

It replaces #116 but builds on the work done. I've only created this new branch because, after rebasing the original onto master, I found myself trying to fix unrelated issues. I trust that is okay with @NullVoxPopuli 🤞🏻

I've had to work around an issue, whereby we can't yield local modifiers. I will circle back and do a bit of refactoring once that's resolved. For now, each sub-component has it's own, name spaced modifier, and references local actions that pass data up to their parent. If anyone knows of a better solution I'd love to hear it.

To pre-empt the question, I've had to make a significant change to the way that <Menu> keeps track of it's items. After implementing the local modifiers, I was seeing a ton of You attempted to update 'items' on 'Menu', but it had already been used previously in the same computation... errors. This makes sense, because items was a tracked property that was being consumed by several getters, and updated by actions in the same compute. To resolve this I've refactored items into a getter that returns an _items array, which is the thing that gets set. Again, if anyone has a better solution please shout. 👍🏻

Test are passing and I've had a good click around the demo app, so fingers crossed this is good to go.

NullVoxPopuli commented 2 years ago

we can't https://github.com/ember-modifier/ember-modifier/issues/78#.

can you expand on this?

This PR is already using local modifiers :upside_down_face: https://github.com/GavinJoyce/ember-headlessui/pull/154/files#diff-435418f0db26ca87ef3a651029c4c8914a5b34f8eeed52a155e97b527ad93771

roomman commented 2 years ago

can you expand on this?

In the original PR, you had a registerItemElement modifier declared in menu/menu-item that yielded down to menu/item-element via a hash. That wasn't working as expected, and I attributed it to this (I figured it was the hash causing the issue.) I've had a second run and it does work, so I will refactor.

Not sure what happened there. 🤔

Nothing to see here. 🤣