basic auth login UI #2316
basic auth login UI #2316
Conversation
components/kflogin/README.md
Outdated
| @@ -0,0 +1,32 @@ | |||
| ## Basic Auth Login Page | |||
|
|
|||
| UI to login kubeflow cluster. | |||
There was a problem hiding this comment.
UI to login into Kubeflow cluster using basic auth
|
/lgtm @yebrahim Would you mind taking a look at this? |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlewi 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 |
components/kflogin/README.md
Outdated
| Launches the test runner in the interactive watch mode.<br> | ||
| See the section about [running tests](https://facebook.github.io/create-react-app/docs/running-tests) for more information. | ||
|
|
||
| ### `npm run build` |
There was a problem hiding this comment.
Should we cleanup this readme? If there are no actual tests (or we're not planning to run them), let's remove the tests section, and we can change the build section below to mention using the makefile instead?
| @@ -0,0 +1,135 @@ | |||
| // This optional code is used to register a service worker. | |||
There was a problem hiding this comment.
You generally won't need this file, it's safe to remove. So are its references in index.js.
| @@ -0,0 +1,32 @@ | |||
| .App { | |||
There was a problem hiding this comment.
This looks like the stock React CSS file, you probably aren't using anything here (.App is used in App.js but it's not doing anything). Can we remove it?
There was a problem hiding this comment.
We need text-align: center;
Maybe leave this file for future style edit?
components/kflogin/src/App.js
Outdated
| import Login from './login'; | ||
| import { createBrowserHistory } from 'history'; | ||
|
|
||
| export const history = createBrowserHistory({ |
There was a problem hiding this comment.
This doesn't seem to be used anywhere.
| @@ -0,0 +1,9 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
I'm on the line about having this stock test, if we're never going to run it. I'm assuming any further developments here will make sure the app loads in the browser, which does more than what this file tests.
components/kflogin/src/index.css
Outdated
| -moz-osx-font-smoothing: grayscale; | ||
| } | ||
|
|
||
| code { |
|
@yebrahim |
|
/lgtm |
|
/cancel hold Thanks @yebrahim |
|
/hold cancel |
* basic auth login UI * add missing steps * remove unused files
* basic auth login UI * add missing steps * remove unused files
Frontend part.
Test passed, ready to merge.
related: #2262
This change is