-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Save login mode into cookie #5246
Conversation
src/app/frontend/login/component.ts
Outdated
@@ -36,7 +36,7 @@ enum LoginModes { | |||
}) | |||
export class LoginComponent implements OnInit { | |||
loginModes = LoginModes; | |||
selectedAuthenticationMode = LoginModes.Kubeconfig; | |||
selectedAuthenticationMode = localStorage.selectedAuthenticationMode; |
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.
Use cookie service and save it in a cookie. Try to avoid using things that are not injected through the constructor. It's harder to follow where they're coming from and it's also harder for the angular build to do a proper tree shaking and other optimizations.
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.
aa4859d
to
f9f7676
Compare
src/app/frontend/login/component.ts
Outdated
@@ -36,7 +37,7 @@ enum LoginModes { | |||
}) | |||
export class LoginComponent implements OnInit { | |||
loginModes = LoginModes; | |||
selectedAuthenticationMode = LoginModes.Kubeconfig; | |||
selectedAuthenticationMode = this.cookies_.get('selectedAuthenticationMode') || ''; |
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.
Do not hardcode the name. Add it to the config and inject into the constructor.
@Inject(CONFIG_DI_TOKEN) private readonly config_: Config,
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
Codecov Report
@@ Coverage Diff @@
## master #5246 +/- ##
=======================================
Coverage 45.51% 45.51%
=======================================
Files 214 214
Lines 10109 10117 +8
Branches 105 107 +2
=======================================
+ Hits 4601 4605 +4
- Misses 5239 5243 +4
Partials 269 269
Continue to review full report at Codecov.
|
/lgtm |
And use it next time if it exists.
fixed tests |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: floreks, maciaszczykm, shu-mutou The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
And use it next time if it exists.
Fixes #5210