ember-basic-dropdown
ember-basic-dropdown copied to clipboard
Shadow DOM support, implements #617
This is a proof-of-concept PR that implements basic Shadow DOM support, closing #617. Please see the issue for explanation of why Shadow DOM may be necessary.
This PR misses Shadow DOM-specific tests and documentation. I would like to receive a general approval and guidelines from ember-basic-dropdown
maintainers before investing moore time into it.
Existing tests pass, and we already use this branch in a project.
This PR does not cover all possible cases of arbitrarily creating Shadow DOM containers and arbitrarily positioning the wormhole container. Supporting all such cases would be tricky and would like require a generic hack like https://github.com/Georgegriff/query-selector-shadow-dom which is undesirable.
But this PR does cover the most basic and demaded case: wrapping your whole Ember app with one Shadow DOM container. It can be done effortlessly with our https://github.com/kaliber5/ember-embedded-snippet/ addon.
In order to avoid leaking app styles into the host website, stylesheets are referenced from inside Shadow DOM. This means that a wormholed dropdown content would appear unstyled if the wormhole is outside of the Shadow DOM.
Thus, a requirement of using ember-basic-dropdown
in Shadow DOM is to set up a wormhole container in application.hbs
and override ['ember-basic-dropdown'].destination
to prevent the addon from emitting a default one into {{content-for "body-footer"}}
.
CC @simonihmig
Thank you for this work. My team has been maintaining an internal fork of the 1.x line with similar shadow DOM support. (We only recently became Octane-ready.)
We were able to upgrade to this fork with no issues.
This PR introduces ember-ref-bucket
to the package.json but doesn't look to be using it. The npm install
also may have been run with npm 7 even though this lib is still using npm 6, as it upgrades the entire package-lock.json to the v2 lockfile format.
Could these npm changes be divorced from the rest of the shadow dom implementation? That might make it easier for upstream to accept the PR.
@lolmaus any luck we can push this forward? Do you need any help with it?
@herzzanu That's a question to addon maintainers. 😬
Thanks @lolmaus
@cibernox this one looks promising. How can we help to move it forward? 🙏
I have a branch here based on the last version and inspired by the changes in this PR https://github.com/EmberExperts/ember-basic-dropdown/tree/shadow-dom
I have a branch here based on the last version and inspired by the changes in this PR https://github.com/EmberExperts/ember-basic-dropdown/tree/shadow-dom
I was able to upgrade to this newer fork with no issues. We've been using the kaliber5 fork for years
Oh I found an issue in the EmberExperts fork. Inside component:basic-dropdown-content
, it still uses the old way of registering the triggerElement
so that handleRootMouseDown
expects it to exist at this.args.triggerElement
. However, the newer version of ember-basic-dropdown no longer uses a component arg. Instead it looks up the trigger element just in time using a data attribute. (Which would need to be shadow DOM aware.)
I'll see if I can get an example commit tomorrow
This commit on top of the EmberExperts fork addresses the issue when clicking the trigger to close the dropdown content.
I scanned for any other dangling document.bleh
element lookups and found 2 more, made sure they are trying out owner.rootElement
as well.
https://github.com/ProsperWorks/ember-basic-dropdown/commit/21d4c7dbe72b2b43e8ba4b999870a30e1ecd6fd6
I would like to close/merge this PR... can sombody bring this PR up to date? Unfortunately there is not anymore mergable...
I've made my fixes to the EmberExpert fork available here https://github.com/EmberExperts/ember-basic-dropdown/pull/1
Maybe @herzzanu can incorporate my changes and open a new PR here?
Thanks for your contribution @apellerano-pw 🙏 I've merged that PR and created a PR here to incorporate the changes back into this repo. We need to fix the conflicts 🙈 and should be good to go.
I see, the changes don't apply cleanly because the addon has been migrated to v2 format. I'm not enough of a git plumber to know the easy way to approach this. I'll try to re-apply our commits onto HEAD and open a PR
edit: see convo here for updates https://github.com/cibernox/ember-basic-dropdown/pull/789#issuecomment-1945159662 tl;dr: v8 is not a trivial port, but the diff applies cleanly to v7. we should consider a v7 branch and doing one more release on v7
closed by #791