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.

Sign up for GitHub

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

Jump to bottom

remove polyfill workarounds #48

Merged
merged 4 commits into from
Feb 20, 2018
Merged

remove polyfill workarounds #48

merged 4 commits into from
Feb 20, 2018

Conversation

tomalec
Copy link
Member

@tomalec tomalec commented Feb 20, 2018

For V0 and V1 quirks

@tomalec tomalec requested a review from alshakero February 20, 2018 13:00
alshakero
alshakero approved these changes Feb 20, 2018
View reviewed changes
Copy link
Contributor

@alshakero alshakero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though three KitchenSink tests fail on TC. They pass locally and manual testing seems to work too.

README.md Outdated
- **V1**: There is also a workaround for [polyfill issue wcjs#872](https://github.com/webcomponents/webcomponentsjs/issues/872)

\*\* **V1**: In IE11 workaround for per-template scripts [polyfill issue wcjs#872](https://github.com/webcomponents/webcomponentsjs/issues/872) does not work
\*\* **V1**: IE11 does not support `currentScript` so bunch of test for useful cases fails
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"IE11 does not support currentScript, so a bunch of tests for useful cases fail."

@tomalec
Copy link
Member Author

tomalec commented Feb 20, 2018

Lucky you, on my machine local tests for KS were failing ;)

@tomalec tomalec merged commit 22c56b9 into master Feb 20, 2018
@tomalec tomalec mentioned this pull request Feb 20, 2018
Closed
@tomalec tomalec deleted the issues/41-remove-x-browser-fix branch February 21, 2018 14:48
@warpech
Copy link
Contributor

warpech commented Feb 21, 2018

It is not ok if tests in KitchenSink fail. In such case, please re-run the tests. If the problem prevails we need to fix it in KitchenSink.

@tomalec
Copy link
Member Author

tomalec commented Feb 22, 2018 •
edited
Loading

Right now all test are failing in FF Nightly, I bet it's due to web driver incompatibilities, and one fails in Chrome due to Polish locale:

3) Failed : KitchenSink.Tests.Test.DatepickerPageTest(Chrome).DatepickerPage_SelectDate
  String lengths are both 7. Strings differ at index 0.
  Expected: "January"
  But was:  "styczen"
  -----------^
at KitchenSink.Tests.Test.DatepickerPageTest.DatepickerPage_SelectDate() in D:\repos\apps\KitchenSink\test\KitchenSink.Tests\Test\DatepickerPageTest.cs:line 37

I don't think it's worth investigating deeper before the demo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alshakero alshakero alshakero approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tomalec @warpech @alshakero