Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ShadowRoot not expected #230

Open
wilianwrech opened this issue May 11, 2020 · 9 comments
Open

ShadowRoot not expected #230

wilianwrech opened this issue May 11, 2020 · 9 comments

Comments

@wilianwrech
Copy link

wilianwrech commented May 11, 2020

Description:

Every time I click to open the dialog an error occurs.

I don't really know how to reproduce, I think there is something to do with view encapsulation in my component, not sure.

@Component({
  ...
  encapsulation: myEncapsulation
})
export class MyComponent {
    ...
}

Error:

ERROR TypeError: "Window.getComputedStyle: Argument 1 does not implement interface Element."
    setDialogPosition ngx-color-picker.es5.js:1838
    openColorPicker ngx-color-picker.es5.js:1681
    Angular 16
    openColorPicker ngx-color-picker.es5.js:1676
    openDialog ngx-color-picker.es5.js:862
    openDialog ngx-color-picker.es5.js:2160
    inputFocus ngx-color-picker.es5.js:2257
    handleFocus ngx-color-picker.es5.js:2066

Solution:

Just fixing the error line will stop the current exception, but there is a lot of other problems using ShadowRoot, the best thing I can think of is to treat the dialog as an inline absolute element and just change its visibility, as it will become part of the Shadow, the problem is gone.

Workaround:

If someone needs a quick solution to the specific error, here is, but i recomend changing the color picker to inline display cpDialogDisplay="inline".

//Hook to support ShadowRoot in getComputedStyle
(function () {
    let originalGetComputedStyle = window.getComputedStyle;
    Object.defineProperty(window, 'getComputedStyle', {
      value: function (...args) {
        try {
          return originalGetComputedStyle.apply(this, args);
        } catch (e) {
          if (e instanceof TypeError && args.length && args[0] instanceof ShadowRoot)
            return window.getComputedStyle(args[0].activeElement, null);
          throw e;
        }
      }
    });
  })();

Version:

I am using v8.2.0, I tried v9.1.0 and the problem persist, I am not using v9.1.0 because of another open issue.

Angular CLI: 7.3.1
Node: 12.14.1
OS: win32 x64
Angular: 7.2.4
... animations, common, compiler, compiler-cli, core, elements
... forms, http, language-service, platform-browser
... platform-browser-dynamic, router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.13.1
@angular-devkit/build-angular     0.13.9
@angular-devkit/build-optimizer   0.13.9
@angular-devkit/build-webpack     0.13.9
@angular-devkit/core              7.3.1
@angular-devkit/schematics        7.3.1
@angular/cli                      7.3.1
@ngtools/webpack                  7.3.9
@schematics/angular               7.3.1
@schematics/update                0.13.1
rxjs                              6.5.5
typescript                        3.2.4
webpack                           4.29.0
@pigeonvictor
Copy link

I Have the exact same problem using encapsulation: ViewEncapsulation.ShadowDom.

This is a problem for the setDialogPosition() function. It contains this loop :

while (node !== null && node.tagName !== 'HTML') {
                style = window.getComputedStyle(node);

When using shadowDom, there is no html tag, so the function fails. Any workaround possible ?

@dimamarksman
Copy link

Why is it closed?

It is still the issue for me.

@wilianwrech
Copy link
Author

Why is it closed?

It is still the issue for me.

I think this will be always an issue, nobody implements things to work across Shadow DOMs, in fact, the purpose of using Shadow DOM is to not mix codes (ref).
You could do a workaround to make this work, but if you are thinking of doing a workaround it's probably better to remove the encapsulation.
In my case, due to bad decisions when creating the project (not mine though), I had to refactor the whole project because everything was using encapsulation, it took some time to do it, but it was the best decision.

@dimamarksman
Copy link

@sconix are you saying that popover creation inside a shadow-dom is a bad idea?
What is wrong to have a popover encapsulated within shadow dom?

@wilianwrech
Copy link
Author

@sconix are you saying that popover creation inside a shadow-dom is a bad idea? What is wrong to have a popover encapsulated within shadow dom?

The thing is, the color picker is being loaded in the DOM that has the Shadow DOM inside it, the purpose of using Shadow DOM is to not have any code from outside running in it. If you create a workaround, you are actually creating a bypass to the Shadow DOM feature, a hole in the encapsulation.

Conclusion: Yes, it's wrong to have a popover working inside a Shadow DOM.

@dimamarksman
Copy link

Ok, thank you for the explanation.
Does it mean if the popover script is loaded directly inside the shadow dom the popover should work within this dom correctly?
Or it's not allowed to use any external scripts inside the shadow?

@wilianwrech
Copy link
Author

Ok, thank you for the explanation. Does it mean if the popover script is loaded directly inside the shadow dom the popover should work within this dom correctly? Or it's not allowed to use any external scripts inside the shadow?

Yes, exactly, everything you want to run inside the encapsulation should be loaded inside it, no matter if it's a style sheet or a script, but keep in mind, if you have hundreds of encapsulated components you will end up loading hundreds of times the same thing, breaking one of the advantages of using a single-page application framework like Angular.

@dimamarksman
Copy link

Even if this package will be loaded directly inside a shadow dom I don't see how the current code will work, the issue will remain the same.
Please correct me if I'm wrong

@wilianwrech
Copy link
Author

Even if this package will be loaded directly inside a shadow dom I don't see how the current code will work, the issue will remain the same. Please correct me if I'm wrong

I didn't try to load it inside the Shadow DOM, if it's not working that way, so maybe it could have some space for improvements. I will reopen this issue so some code maintainer can see if something can be done.
This issue is from 2 years ago, and I am not using it anymore, so I don't want to invest time to fix this.

@wilianwrech wilianwrech reopened this Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants