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

Examples not working under Edge / 15 #139

Open
robcole opened this issue Aug 29, 2017 · 14 comments
Open

Examples not working under Edge / 15 #139

robcole opened this issue Aug 29, 2017 · 14 comments

Comments

@robcole
Copy link

robcole commented Aug 29, 2017

Using a cross-browser testing app to test a production app, however, this is also replicable in our local examples. Attaching screenshots of one in Chrome, other in Edge / 15 (similar issues exist in Edge / 14 and IE / 11).

Happy to debug more if someone can give me an area to look; it's looking like support for both IE and Edge is lacking.

My hunch is that this is due to the polyfills, but I haven't tracked down a way to actually gain any traction fixing it yet.


Example: http://snuggsi.herokuapp.com/examples/hello-world

Also tested: JS fiddle of example, http://jsfiddle.net/snuggs/3zwkj9wg/?utm_source=website&utm_medium=embed&utm_campaign=3zwkj9wg

Chrome

image

Edge / 15

image


Console Errors (Edge)

image

@snuggs
Copy link
Member

snuggs commented Aug 30, 2017

@robcole seems like this is the "gold standard" at least 2 years ago. https://blogs.windows.com/msedgedev/2015/07/15/microsoft-edge-and-web-components/

@snuggs
Copy link
Member

snuggs commented Aug 30, 2017

@robcole
#127

@robcole
Copy link
Author

robcole commented Aug 31, 2017

@snuggs @brandondees - I was able to get the page load working with a new Polyfill (link) but there's a new issue with this.context not being defined. Tried accessing it in an onconnect function with no luck - small snippet of code + screenshot of error posted; sorry for screenshot, but can't copy/paste from Browserstack.

debugger call inserted so I can log what this and this.context look like before the error is triggered (on next line).

const supporterWallElement = class extends HTMLElement {
  initialize () {
    this.context.currentPage = 1; // pagination
    this.context.supporters = [];

    this.context.wallUrl = document
      .querySelector(`supporter-wall`)
      .dataset
      .wallUrl;
  }

  onconnect () {
    const template = Template(`supportercard`);
    debugger;
    template.bind(this.context.supporters);
    this.fetchSupporters();
  }
...

image

@brandondees
Copy link
Collaborator

I don't think this.context has any magic to it, just a convention pattern you can follow. basically, set it to an empty object yourself. right, @snuggs ?

@robcole
Copy link
Author

robcole commented Aug 31, 2017

@brandondees What I mean by this is that this.context isn't being initialized in Edge the way it is in Chrome in the Component class, i.e. https://github.com/devpunks/snuggsi/blob/master/dist/snuggsi.js#L341 doesn't appear to be triggered.

@snuggs
Copy link
Member

snuggs commented Aug 31, 2017

AHHH I See interesting @robcole that Polyfill is part of the solution we are coming up with. Thanks man!!!! @robcole /cc @brandondees

@snuggs
Copy link
Member

snuggs commented Sep 1, 2017

Promising!!! Feelin' real good about this 💪 @robcole @brandondees @albertoponti @cristhiandick @angelocordon @codeSnorter
capture d ecran 2017-09-01 a 06 49 54

@snuggs
Copy link
Member

snuggs commented Sep 1, 2017

Internet Explorer 11

capture d ecran 2017-09-01 a 09 28 36

Firefox 54

capture d ecran 2017-09-01 a 09 35 28

@snuggs
Copy link
Member

snuggs commented Sep 5, 2017

@brandondees @robcole @tmornini @kurtcagle I have been through (I.E.) Hell and back. Was an extremely dark place the past few days in IE land. (Couldn't even log on Slack let alone messenger due to resource hogging of Parallels). That said... Polyfill complete (from my testing). Big salute to @robcole for coming through with that document.registerElement polyfill as well. It filled in the missing pieces that was giving webcomponentsjs polyfill problems in IE. Also integrating into snuggsi.es so we can remove the polyfill noise from the README (confusing as fuck! and @cristhiandick hates it) . Which means we have custom elements in IE (back to 8 I believe) AND Firefox. That said, now getting snuggsi upgrades working which we have full control over. Still down by 1 in the football game. As soon as we get those undefined clear that's the 2 point conversion to win the Super Bowl.

/cc @albertoponti @cristhiandick 🏈 💯 🎉 🥅

Polyfill Implementation

HTMLElement

CustomElementRegistry

Screen Captures

Safari

capture d ecran 2017-09-05 a 09 47 31

Firefox

capture d ecran 2017-09-05 a 09 46 10

Chrome

capture d ecran 2017-09-05 a 09 45 40

Internet Explorer 11

capture d ecran 2017-09-05 a 09 37 16

@robcole
Copy link
Author

robcole commented Sep 5, 2017

Thanks for the updates @snuggs - LMK when you're at a point where you'd like some real-world testers and I'd be happy to test against our current code! 🤞

@snuggs
Copy link
Member

snuggs commented Sep 12, 2017

Sheds light on solution. From the spec maintainers. And their frustration with polyfill authors whatwg/html#1704 Particularly whatwg/html#1704 (comment)

@snuggs
Copy link
Member

snuggs commented Sep 12, 2017

@robcole @brandondees @albertoponti @cristhiandick.

Only 1 bug left in FF and all clear. I'd say 98% complete cross browser.

snuggsi

@snuggs
Copy link
Member

snuggs commented Sep 12, 2017

... Sans webcomponentsjs pollyfill

@brandondees and it's getting worse... Good thing we got off the boat. github.com/webcomponents/webcomponentsjs/issues/830#issuecomment-328973451

@snuggs
Copy link
Member

snuggs commented Sep 13, 2017

@robcole @brandondees @albertoponti @cristhiandick iOS & (as @robcole calls it) IE (Edge) #Touchdown 🏈 with 1% to go. ;-)

capture d ecran 2017-09-13 a 19 47 31

img_4154

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

3 participants