Merge pull request #30 from mabashian/update-pf-fix-login

Updates to login page after LoginPage and LoginForm components landed
2026-03-14 23:47:28 -02:30 · 2018-12-03 12:06:49 -05:00
parent 07d2a1ed1e 8f6b476388
commit 1e7ab9deed
5 changed files with 123 additions and 149 deletions
--- a/tests/pages/Login.jsx
+++ b/tests/pages/Login.jsx
@@ -2,33 +2,31 @@ import React from 'react';
 import { MemoryRouter } from 'react-router-dom';
 import { mount, shallow } from 'enzyme';
 import { asyncFlush } from '../../jest.setup';
-import LoginPage from '../../src/pages/Login';
+import AtLogin from '../../src/pages/Login';
 import api from '../../src/api';

-const LOGIN_ERROR_MESSAGE = 'Invalid username or password. Please try again.';
-
-describe('<LoginPage />', () => {
+describe('<Login />', () => {
  let loginWrapper;
+  let atLogin;
  let loginPage;
  let loginForm;
  let usernameInput;
  let passwordInput;
-  let errorTextArea;
  let submitButton;
-  let defaultLogo;
+  let loginHeaderLogo;

  const findChildren = () => {
+    atLogin = loginWrapper.find('AtLogin');
    loginPage = loginWrapper.find('LoginPage');
-    loginForm = loginWrapper.find('form.pf-c-form');
-    usernameInput = loginWrapper.find('.pf-c-form__group#username TextInput');
-    passwordInput = loginWrapper.find('.pf-c-form__group#password TextInput');
-    errorTextArea = loginWrapper.find('.pf-c-form__helper-text.pf-m-error');
+    loginForm = loginWrapper.find('LoginForm');
+    usernameInput = loginWrapper.find('input#pf-login-username-id');
+    passwordInput = loginWrapper.find('input#pf-login-password-id');
    submitButton = loginWrapper.find('Button[type="submit"]');
-    defaultLogo = loginWrapper.find('TowerLogo');
+    loginHeaderLogo = loginWrapper.find('LoginHeaderBrand Brand');
  };

  beforeEach(() => {
-    loginWrapper = mount(<MemoryRouter><LoginPage /></MemoryRouter>);
+    loginWrapper = mount(<MemoryRouter><AtLogin /></MemoryRouter>);
    findChildren();
  });

@@ -38,62 +36,78 @@ describe('<LoginPage />', () => {

  test('initially renders without crashing', () => {
    expect(loginWrapper.length).toBe(1);
+    expect(loginPage.length).toBe(1);
    expect(loginForm.length).toBe(1);
    expect(usernameInput.length).toBe(1);
    expect(usernameInput.props().value).toBe('');
    expect(passwordInput.length).toBe(1);
    expect(passwordInput.props().value).toBe('');
-    expect(errorTextArea.length).toBe(1);
-    expect(loginPage.state().error).toBe('');
+    expect(atLogin.state().isValidPassword).toBe(true);
    expect(submitButton.length).toBe(1);
    expect(submitButton.props().isDisabled).toBe(false);
-    expect(defaultLogo.length).toBe(1);
+    expect(loginHeaderLogo.length).toBe(1);
  });

-  test('custom logo renders Brand component', () => {
-    loginWrapper = mount(<MemoryRouter><LoginPage logo="hey" /></MemoryRouter>);
+  test('custom logo renders Brand component with correct src and alt', () => {
+    loginWrapper = mount(<MemoryRouter><AtLogin logo="images/foo.jpg" alt="Foo Application" /></MemoryRouter>);
    findChildren();
-    expect(defaultLogo.length).toBe(0);
+    expect(loginHeaderLogo.length).toBe(1);
+    expect(loginHeaderLogo.props().src).toBe('data:image/jpeg;images/foo.jpg');
+    expect(loginHeaderLogo.props().alt).toBe('Foo Application');
+  });
+
+  test('default logo renders Brand component with correct src and alt', () => {
+    loginWrapper = mount(<MemoryRouter><AtLogin /></MemoryRouter>);
+    findChildren();
+    expect(loginHeaderLogo.length).toBe(1);
+    expect(loginHeaderLogo.props().src).toBe('tower-logo-header.svg');
+    expect(loginHeaderLogo.props().alt).toBe('Ansible Tower');
  });

  test('state maps to un/pw input value props', () => {
-    loginPage.setState({ username: 'un', password: 'pw' });
-    expect(loginPage.state().username).toBe('un');
-    expect(loginPage.state().password).toBe('pw');
+    atLogin.setState({ username: 'un', password: 'pw' });
+    expect(atLogin.state().username).toBe('un');
+    expect(atLogin.state().password).toBe('pw');
    findChildren();
    expect(usernameInput.props().value).toBe('un');
    expect(passwordInput.props().value).toBe('pw');
  });

  test('updating un/pw clears out error', () => {
-    loginPage.setState({ error: 'error!' });
-    usernameInput.instance().props.onChange('uname');
-    expect(loginPage.state().username).toBe('uname');
-    expect(loginPage.state().error).toBe('');
-    loginPage.setState({ error: 'error!' });
-    passwordInput.instance().props.onChange('pword');
-    expect(loginPage.state().password).toBe('pword');
-    expect(loginPage.state().error).toBe('');
+    atLogin.setState({ isValidPassword: false });
+    expect(loginWrapper.find('.pf-c-form__helper-text.pf-m-error').length).toBe(1);
+    usernameInput.instance().value = 'uname';
+    usernameInput.simulate('change');
+    expect(atLogin.state().username).toBe('uname');
+    expect(atLogin.state().isValidPassword).toBe(true);
+    expect(loginWrapper.find('.pf-c-form__helper-text.pf-m-error').length).toBe(0);
+    atLogin.setState({ isValidPassword: false });
+    expect(loginWrapper.find('.pf-c-form__helper-text.pf-m-error').length).toBe(1);
+    passwordInput.instance().value = 'pword';
+    passwordInput.simulate('change');
+    expect(atLogin.state().password).toBe('pword');
+    expect(atLogin.state().isValidPassword).toBe(true);
+    expect(loginWrapper.find('.pf-c-form__helper-text.pf-m-error').length).toBe(0);
  });

  test('api.login not called when loading', () => {
    api.login = jest.fn().mockImplementation(() => Promise.resolve({}));
-    expect(loginPage.state().loading).toBe(false);
-    loginPage.setState({ loading: true });
-    submitButton.simulate('submit');
+    expect(atLogin.state().loading).toBe(false);
+    atLogin.setState({ loading: true });
+    submitButton.simulate('click');
    expect(api.login).toHaveBeenCalledTimes(0);
  });

  test('submit calls api.login successfully', async () => {
    api.login = jest.fn().mockImplementation(() => Promise.resolve({}));
-    expect(loginPage.state().loading).toBe(false);
-    loginPage.setState({ username: 'unamee', password: 'pwordd' });
-    submitButton.simulate('submit');
+    expect(atLogin.state().loading).toBe(false);
+    atLogin.setState({ username: 'unamee', password: 'pwordd' });
+    submitButton.simulate('click');
    expect(api.login).toHaveBeenCalledTimes(1);
    expect(api.login).toHaveBeenCalledWith('unamee', 'pwordd');
-    expect(loginPage.state().loading).toBe(true);
+    expect(atLogin.state().loading).toBe(true);
    await asyncFlush();
-    expect(loginPage.state().loading).toBe(false);
+    expect(atLogin.state().loading).toBe(false);
  });

  test('submit calls api.login handles 401 error', async () => {
@@ -102,15 +116,16 @@ describe('<LoginPage />', () => {
      err.response = { status: 401, message: 'problem' };
      return Promise.reject(err);
    });
-    expect(loginPage.state().loading).toBe(false);
-    loginPage.setState({ username: 'unamee', password: 'pwordd' });
-    submitButton.simulate('submit');
+    expect(atLogin.state().loading).toBe(false);
+    expect(atLogin.state().isValidPassword).toBe(true);
+    atLogin.setState({ username: 'unamee', password: 'pwordd' });
+    submitButton.simulate('click');
    expect(api.login).toHaveBeenCalledTimes(1);
    expect(api.login).toHaveBeenCalledWith('unamee', 'pwordd');
-    expect(loginPage.state().loading).toBe(true);
+    expect(atLogin.state().loading).toBe(true);
    await asyncFlush();
-    expect(loginPage.state().error).toBe(LOGIN_ERROR_MESSAGE);
-    expect(loginPage.state().loading).toBe(false);
+    expect(atLogin.state().isValidPassword).toBe(false);
+    expect(atLogin.state().loading).toBe(false);
  });

  test('submit calls api.login handles non-401 error', async () => {
@@ -119,21 +134,20 @@ describe('<LoginPage />', () => {
      err.response = { status: 500, message: 'problem' };
      return Promise.reject(err);
    });
-    expect(loginPage.state().loading).toBe(false);
-    loginPage.setState({ username: 'unamee', password: 'pwordd' });
-    submitButton.simulate('submit');
+    expect(atLogin.state().loading).toBe(false);
+    atLogin.setState({ username: 'unamee', password: 'pwordd' });
+    submitButton.simulate('click');
    expect(api.login).toHaveBeenCalledTimes(1);
    expect(api.login).toHaveBeenCalledWith('unamee', 'pwordd');
-    expect(loginPage.state().loading).toBe(true);
+    expect(atLogin.state().loading).toBe(true);
    await asyncFlush();
-    expect(loginPage.state().error).toBe('');
-    expect(loginPage.state().loading).toBe(false);
+    expect(atLogin.state().loading).toBe(false);
  });

  test('render Redirect to / when already authenticated', () => {
    api.isAuthenticated = jest.fn();
    api.isAuthenticated.mockReturnValue(true);
-    loginWrapper = shallow(<LoginPage />);
+    loginWrapper = shallow(<AtLogin />);
    const redirectElem = loginWrapper.find('Redirect');
    expect(redirectElem.length).toBe(1);
    expect(redirectElem.props().to).toBe('/');
--- a/package-lock.json
+++ b/package-lock.json
@@ -977,26 +977,33 @@
      "integrity": "sha512-rLu3wcBWH4P5q1CGoSSH/i9hrXs7SlbRLkoq9IGuoPYNGQvDJ3pt/wmOM+XgYjIDRMVIdkUWt0RsfzF50JfnCw=="
    },
    "@patternfly/patternfly-next": {
-      "version": "1.0.64",
-      "resolved": "https://registry.npmjs.org/@patternfly/patternfly-next/-/patternfly-next-1.0.64.tgz",
-      "integrity": "sha512-Pxug4QU6r5kB1/dSSJJgn8h5tDmzPennJgL9s52ls4TDuDStxgbY4N8ADWEVRuAEh1w9vRo2TdAvJNpUVVoC7w=="
+      "version": "1.0.84",
+      "resolved": "https://registry.npmjs.org/@patternfly/patternfly-next/-/patternfly-next-1.0.84.tgz",
+      "integrity": "sha512-sIRfo/tk4NSnaRwHIHLUf4XoqzNNa4MMa8ZWivzzSfdZ5pCbgvZtyEUqKnQAEH6zuaCM9S8HyhxcNXnm/xaYaQ=="
    },
    "@patternfly/react-core": {
-      "version": "1.28.4",
-      "resolved": "https://registry.npmjs.org/@patternfly/react-core/-/react-core-1.28.4.tgz",
-      "integrity": "sha512-Jp8fILx0Vz8ZpfyTbOXwjNCNc2MMciWgkz7nYN+qeBrvAKbYaRIaWlSafbVK7ySoM+trmGDJ67p3N+f5eE0eqQ==",
+      "version": "1.37.2",
+      "resolved": "https://registry.npmjs.org/@patternfly/react-core/-/react-core-1.37.2.tgz",
+      "integrity": "sha512-zzHwqGEsRWzw9uRkbrf6PmUpcl6EMxQSbUJ1zmv7Ryc32CcSMgrDL4ZA3x/tf4TAYTMRBKUK3O8S5veRjxpFuw==",
      "requires": {
-        "@patternfly/react-icons": "^2.6.0",
+        "@patternfly/react-icons": "^2.9.1",
        "@patternfly/react-styles": "^2.3.0",
        "@patternfly/react-tokens": "^1.0.0",
        "exenv": "^1.2.2",
        "focus-trap-react": "^4.0.1"
+      },
+      "dependencies": {
+        "@patternfly/react-icons": {
+          "version": "2.9.1",
+          "resolved": "https://registry.npmjs.org/@patternfly/react-icons/-/react-icons-2.9.1.tgz",
+          "integrity": "sha512-CBTpGXvqr91rBpxeb5/l2BimrtRlMkBKnIOTgX7V44MIIq3YE3P6A6CQK0fgIH1HGvCdiNf5sXbQz9xp+pB/3A=="
+        }
      }
    },
    "@patternfly/react-icons": {
-      "version": "2.6.0",
-      "resolved": "https://registry.npmjs.org/@patternfly/react-icons/-/react-icons-2.6.0.tgz",
-      "integrity": "sha512-r731Huakc4bm5RdrFGX1SWN10vXFaSBSyuDfa6lva6njqe3ZxAf9Q0NLBw3b2l8oe6Rpqw0Ru6PCyzuzqkXlcQ=="
+      "version": "2.9.1",
+      "resolved": "https://registry.npmjs.org/@patternfly/react-icons/-/react-icons-2.9.1.tgz",
+      "integrity": "sha512-CBTpGXvqr91rBpxeb5/l2BimrtRlMkBKnIOTgX7V44MIIq3YE3P6A6CQK0fgIH1HGvCdiNf5sXbQz9xp+pB/3A=="
    },
    "@patternfly/react-styles": {
      "version": "2.3.0",
@@ -1018,9 +1025,9 @@
      }
    },
    "@patternfly/react-tokens": {
-      "version": "1.6.0",
-      "resolved": "https://registry.npmjs.org/@patternfly/react-tokens/-/react-tokens-1.6.0.tgz",
-      "integrity": "sha512-2JSfarzg/OV04SBrEpD7NKvJ27z8gpBcQXoXgdf0VpnvGKceVKOnTm2NLbG4Gm6WJNMtyKGuUw1afTdFoHPl3w=="
+      "version": "1.9.0",
+      "resolved": "https://registry.npmjs.org/@patternfly/react-tokens/-/react-tokens-1.9.0.tgz",
+      "integrity": "sha512-wxlxeY5B37FkI9W3x4EQyZ9Q8lra3xBYEUg5CFCmWQZTvdH4vAC19l7mE+AQZqHXD4unvltS0ndi753LeHPyAg=="
    },
    "@types/node": {
      "version": "10.12.1",
--- a/package.json
+++ b/package.json
@@ -40,11 +40,11 @@
    "webpack-dev-server": "^3.1.4"
  },
  "dependencies": {
-    "@patternfly/patternfly-next": "^1.0.64",
-    "@patternfly/react-core": "^1.28.1",
-    "@patternfly/react-icons": "^2.6.0",
+    "@patternfly/patternfly-next": "^1.0.84",
+    "@patternfly/react-core": "^1.37.2",
+    "@patternfly/react-icons": "^2.9.1",
    "@patternfly/react-styles": "^2.3.0",
-    "@patternfly/react-tokens": "^1.6.0",
+    "@patternfly/react-tokens": "^1.9.0",
    "axios": "^0.18.0",
    "react": "^16.4.1",
    "react-dom": "^16.4.1",
--- a/src/app.scss
+++ b/src/app.scss
@@ -21,7 +21,7 @@
 //

 .pf-l-page__sidebar{
-  --pf-l-page__sidebar--Width--lg: 255px;
+  --pf-l-page__sidebar--md--Width: 255px;

  .pf-c-nav {
    overflow-y: auto;
--- a/src/pages/Login.jsx
+++ b/src/pages/Login.jsx
@@ -1,33 +1,22 @@
 import React, { Component } from 'react';
 import { Redirect } from 'react-router-dom';
 import {
-  Brand,
-  Button,
-  Level,
-  LevelItem,
-  Login,
-  LoginBox,
-  LoginBoxHeader,
-  LoginBoxBody,
-  LoginFooter,
-  LoginHeaderBrand,
-  TextInput,
+  LoginForm,
+  LoginPage,
 } from '@patternfly/react-core';

-import TowerLogo from '../components/TowerLogo';
+import towerLogo from '../../images/tower-logo-header.svg';
 import api from '../api';

-const LOGIN_ERROR_MESSAGE = 'Invalid username or password. Please try again.';
-
-class LoginPage extends Component {
+class AtLogin extends Component {
  constructor (props) {
    super(props);

    this.state = {
      username: '',
      password: '',
-      error: '',
-      loading: false,
+      isValidPassword: true,
+      loading: false
    };
  }

@@ -37,9 +26,9 @@ class LoginPage extends Component {

  safeSetState = obj => !this.unmounting && this.setState(obj);

-  handleUsernameChange = value => this.safeSetState({ username: value, error: '' });
+  handleUsernameChange = value => this.safeSetState({ username: value, isValidPassword: true });

-  handlePasswordChange = value => this.safeSetState({ password: value, error: '' });
+  handlePasswordChange = value => this.safeSetState({ password: value, isValidPassword: true });

  handleSubmit = async event => {
    const { username, password, loading } = this.state;
@@ -53,7 +42,7 @@ class LoginPage extends Component {
        await api.login(username, password);
      } catch (error) {
        if (error.response.status === 401) {
-          this.safeSetState({ error: LOGIN_ERROR_MESSAGE });
+          this.safeSetState({ isValidPassword: false });
        }
      } finally {
        this.safeSetState({ loading: false });
@@ -62,75 +51,39 @@ class LoginPage extends Component {
  }

  render () {
-    const { username, password, loading, error } = this.state;
-    const { logo, loginInfo } = this.props;
+    const { username, password, isValidPassword } = this.state;
+    const { logo, alt } = this.props;
+    const logoSrc = logo ? `data:image/jpeg;${logo}` : towerLogo;
+    const logoAlt = alt || 'Ansible Tower';
+    const LOGIN_ERROR_MESSAGE = 'Invalid username or password. Please try again.';
+    const LOGIN_TITLE = 'Welcome to Ansible Tower! Please Sign In.';
+    const LOGIN_USERNAME = 'Username';
+    const LOGIN_PASSWORD = 'Password';

    if (api.isAuthenticated()) {
      return (<Redirect to="/" />);
    }

    return (
-      <Login
-        header={(
-          <LoginHeaderBrand>
-            {logo ? <Brand src={`data:image/jpeg;${logo}`} alt="logo brand" /> : <TowerLogo />}
-          </LoginHeaderBrand>
-        )}
-        footer={<LoginFooter>{ loginInfo }</LoginFooter>}
+      <LoginPage
+        mainBrandImgSrc={logoSrc}
+        mainBrandImgAlt={logoAlt}
+        loginTitle={LOGIN_TITLE}
      >
-        <LoginBox>
-          <LoginBoxHeader>
-            Welcome to Ansible Tower! Please Sign In.
-          </LoginBoxHeader>
-          <LoginBoxBody>
-            <form className="pf-c-form" onSubmit={this.handleSubmit}>
-              <div className="pf-c-form__group" id="username">
-                <label className="pf-c-form__label" htmlFor="username">
-                  Username
-                  <span className="pf-c-form__label__required" aria-hidden="true">&#42;</span>
-                </label>
-                <TextInput
-                  autoComplete="off"
-                  aria-label="Username"
-                  name="username"
-                  type="text"
-                  isDisabled={loading}
-                  value={username}
-                  onChange={this.handleUsernameChange}
-                />
-              </div>
-              <div className="pf-c-form__group" id="password">
-                <label className="pf-c-form__label" htmlFor="pw">
-                  Password
-                  <span className="pf-c-form__label__required" aria-hidden="true">&#42;</span>
-                </label>
-                <TextInput
-                  aria-label="Password"
-                  name="password"
-                  type="password"
-                  isDisabled={loading}
-                  value={password}
-                  onChange={this.handlePasswordChange}
-                />
-              </div>
-              <Level>
-                <LevelItem>
-                  <p className="pf-c-form__helper-text pf-m-error" aria-live="polite">
-                    { error }
-                  </p>
-                </LevelItem>
-                <LevelItem>
-                  <Button type="submit" isDisabled={loading}>
-                    Sign In
-                  </Button>
-                </LevelItem>
-              </Level>
-            </form>
-          </LoginBoxBody>
-        </LoginBox>
-      </Login>
+        <LoginForm
+          usernameLabel={LOGIN_USERNAME}
+          usernameValue={username}
+          onChangeUsername={this.handleUsernameChange}
+          passwordLabel={LOGIN_PASSWORD}
+          passwordValue={password}
+          onChangePassword={this.handlePasswordChange}
+          isValidPassword={isValidPassword}
+          passwordHelperTextInvalid={LOGIN_ERROR_MESSAGE}
+          onLoginButtonClick={this.handleSubmit}
+        />
+      </LoginPage>
    );
  }
 }

-export default LoginPage;
+export default AtLogin;