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

IE11 Auth Code Sample #1883

Merged
merged 15 commits into from Jul 6, 2020
Merged

IE11 Auth Code Sample #1883

merged 15 commits into from Jul 6, 2020

Conversation

tnorling
Copy link
Collaborator

@tnorling tnorling commented Jul 2, 2020

No description provided.

@github-actions github-actions bot added the samples Related to the samples apps for the library. label Jul 2, 2020
pkanher617
pkanher617 approved these changes Jul 2, 2020
View reviewed changes
Copy link
Contributor

@pkanher617 pkanher617 left a comment

Choose a reason for hiding this comment

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

Looks fine, one nitpick is I think the folder name should be lower-case

jasonnutter
jasonnutter suggested changes Jul 2, 2020
View reviewed changes
Copy link
Contributor

@jasonnutter jasonnutter left a comment •
edited

Choose a reason for hiding this comment

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

A couple comments. Can you add a README as well that details the steps that are needed to support IE11 (add promise Polyfill, setAuthStateInCookie, redirects instead of popups, etc)?

samples/msal-browser-samples/VanillaJSTestApp2.0/app/IE11/index.html Outdated Show resolved Hide resolved
samples/msal-browser-samples/VanillaJSTestApp2.0/app/IE11/graph.js Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jul 2, 2020 •
edited

Coverage Status

Coverage remained the same at 80.988% when pulling f52633f on ie11-sample into e799ea5 on dev.

@pkanher617
Copy link
Contributor

Need to add a test folder with a sample e2e test spec

@tnorling
Copy link
Collaborator Author

tnorling commented Jul 2, 2020 •
edited

Need to add a test folder with a sample e2e test spec

I'm not sure there's a value add for E2E tests on this sample since the test framework uses Chromium. Since the build is failing because of it, we should figure out how to make it fail gracefully.

Edit: Updated test runner to ignore samples without a test folder

@tnorling tnorling merged commit 97e8cec into dev Jul 6, 2020
@tnorling tnorling deleted the ie11-sample branch July 6, 2020 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonnutter jasonnutter jasonnutter approved these changes

@pkanher617 pkanher617 pkanher617 approved these changes

@DarylThayil DarylThayil Awaiting requested review from DarylThayil

@jmckennon jmckennon Awaiting requested review from jmckennon

@jo-arroyo jo-arroyo Awaiting requested review from jo-arroyo jo-arroyo is a code owner

@sameerag sameerag Awaiting requested review from sameerag sameerag is a code owner

@hectormmg hectormmg Awaiting requested review from hectormmg hectormmg is a code owner

Assignees
No one assigned
Labels
samples Related to the samples apps for the library.
Projects
None yet
Milestone
@azure/msal-browser@2.0.0 - GA Release
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tnorling @coveralls @pkanher617 @jasonnutter