-
Notifications
You must be signed in to change notification settings - Fork 268
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
Add idps option for config for Signin widget flow #572
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
would you mind adding a selenium test verifying idp window opens on idp button click?
f421e0e
to
fb192e8
Compare
Codecov Report
@@ Coverage Diff @@
## master #572 +/- ##
==========================================
+ Coverage 92.96% 93.70% +0.73%
==========================================
Files 38 42 +4
Lines 2161 3017 +856
Branches 452 711 +259
==========================================
+ Hits 2009 2827 +818
- Misses 152 190 +38
Continue to review full report at Codecov.
|
idps: config.idps.split(/\s+/).map(v => { | ||
const parts = v.split(/:/); | ||
if (parts.length == 2) { | ||
return {type: parts[0], id: parts[1]}; | ||
} | ||
return null; | ||
}).filter(v => v) | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: (very nit). I like to keep the visual focus on the relevant parts of code to make it easier for the next coder who is skimming over the code and may not be familiar with it. What do you think of this:
config.idps.split(/\s+/).map(idpToken => {
const [type, id] = idpToken.split(/:/);
if(!type || !id) {
return null;
}
return { type, id};
}).filter(v => v);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
b46a430
to
a4abac3
Compare
a4abac3
to
af85bde
Compare
Added
idps
to for sample config UI for Signin Widget flow.