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

In SSO mode, hide the password input instead of removing it from the DOM #1407

Merged
merged 3 commits into from
Jun 21, 2018
Merged

In SSO mode, hide the password input instead of removing it from the DOM #1407

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9b3368
In SSO mode, hide the password input instead of removing it from the DOM
luisrudge Jun 12, 2018
7687e41
Fix test
luisrudge Jun 12, 2018
fb4edad
Merge branch 'master' into fix-sso-removing-password-field
luisrudge Jun 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions css/index.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1361,3 +1361,6 @@ tabsHeight = 40px

input[type="button"]
cursor pointer

.auth0-lock-hidden
display: none
14 changes: 14 additions & 0 deletions src/__tests__/field/__snapshots__/login_pane.test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ exports[`LoginPane hides password link when lock does not have the screen \`forg
/>
<div
data-__type="password_pane"
data-hidden={false}
data-i18n={Object {}}
data-lock={Object {}}
data-placeholder="passwordInputPlaceholder"
Expand All @@ -31,6 +32,7 @@ exports[`LoginPane hides password link when showForgotPasswordLink === false 1`]
/>
<div
data-__type="password_pane"
data-hidden={false}
data-i18n={Object {}}
data-lock={Object {}}
data-placeholder="passwordInputPlaceholder"
Expand All @@ -48,6 +50,13 @@ exports[`LoginPane hides password pane when showPassword===false 1`] = `
data-usernameStyle={undefined}
data-validateFormat={false}
/>
<div
data-__type="password_pane"
data-hidden={true}
data-i18n={Object {}}
data-lock={Object {}}
data-placeholder="passwordInputPlaceholder"
/>
<p
className="auth0-lock-alternative"
>
Expand All @@ -74,6 +83,7 @@ exports[`LoginPane renders correctly 1`] = `
/>
<div
data-__type="password_pane"
data-hidden={false}
data-i18n={Object {}}
data-lock={Object {}}
data-placeholder="passwordInputPlaceholder"
Expand Down Expand Up @@ -103,6 +113,7 @@ exports[`LoginPane shows email pane when user usernameStyle === email 1`] = `
/>
<div
data-__type="password_pane"
data-hidden={false}
data-i18n={Object {}}
data-lock={Object {}}
data-placeholder="passwordInputPlaceholder"
Expand Down Expand Up @@ -136,6 +147,7 @@ exports[`LoginPane shows header when instructions is not empty 1`] = `
/>
<div
data-__type="password_pane"
data-hidden={false}
data-i18n={Object {}}
data-lock={Object {}}
data-placeholder="passwordInputPlaceholder"
Expand Down Expand Up @@ -166,6 +178,7 @@ exports[`LoginPane shows username pane when user usernameStyle !== email 1`] = `
/>
<div
data-__type="password_pane"
data-hidden={false}
data-i18n={Object {}}
data-lock={Object {}}
data-placeholder="passwordInputPlaceholder"
Expand Down Expand Up @@ -196,6 +209,7 @@ exports[`LoginPane shows username pane when user usernameStyle !== email 2`] = `
/>
<div
data-__type="password_pane"
data-hidden={false}
data-i18n={Object {}}
data-lock={Object {}}
data-placeholder="passwordInputPlaceholder"
Expand Down
20 changes: 20 additions & 0 deletions src/__tests__/field/__snapshots__/password_pane.test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,26 @@ exports[`PasswordPane disables input when submitting 1`] = `
</div>
`;

exports[`PasswordPane renders correct css className when \`hidden\` is true 1`] = `
<div
className="auth0-lock-input-block auth0-lock-input-show-password auth0-lock-hidden"
>
<div
data-__type="password_input"
data-disabled={false}
data-invalidHint="blankErrorHint"
data-isValid={false}
data-onChange={[Function]}
data-placeholder="placeholder"
data-policy="policy"
data-showPassword="showPassword"
data-showPasswordStrengthMessage={false}
data-strengthMessages={Object {}}
data-value="password"
/>
</div>
`;

exports[`PasswordPane renders correctly 1`] = `
<div
className="auth0-lock-input-block auth0-lock-input-show-password"
Expand Down
4 changes: 4 additions & 0 deletions src/__tests__/field/password_pane.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ describe('PasswordPane', () => {
const PasswordPane = getComponent();
expectComponent(<PasswordPane {...defaultProps} />).toMatchSnapshot();
});
it('renders correct css className when `hidden` is true', () => {
const PasswordPane = getComponent();
expectComponent(<PasswordPane {...defaultProps} hidden />).toMatchSnapshot();
});
it('disables input when submitting', () => {
require('core/index').submitting = () => true;
const PasswordPane = getComponent();
Expand Down
11 changes: 6 additions & 5 deletions src/connection/database/login_pane.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ export default class LoginPane extends React.Component {
/>
);

const passwordPane = showPassword ? (
<PasswordPane i18n={i18n} lock={lock} placeholder={passwordInputPlaceholder} />
) : null;

const dontRememberPassword =
showForgotPasswordLink && hasScreen(lock, 'forgotPassword') ? (
<p className="auth0-lock-alternative">
Expand All @@ -70,7 +66,12 @@ export default class LoginPane extends React.Component {
<div>
{header}
{fieldPane}
{passwordPane}
<PasswordPane
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you switching from using a const? Not a blocker, just curious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the const was a conditional. It was adding/removing the input from the DOM based on that condition. This triggered the bug itself. So, now, it will always have the input, it will only use css to hide the input

i18n={i18n}
lock={lock}
placeholder={passwordInputPlaceholder}
hidden={!showPassword}
/>
{dontRememberPassword}
</div>
);
Expand Down
8 changes: 5 additions & 3 deletions src/field/password/password_pane.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ export default class PasswordPane extends React.Component {
};

render() {
const { i18n, lock, placeholder, policy, strengthMessages } = this.props;
const { i18n, lock, placeholder, policy, strengthMessages, hidden } = this.props;
const hiddenCss = hidden ? ' auth0-lock-hidden' : '';
return (
<div className="auth0-lock-input-block auth0-lock-input-show-password">
<div className={`auth0-lock-input-block auth0-lock-input-show-password${hiddenCss}`}>
<PasswordInput
value={c.getFieldValue(lock, 'password')}
invalidHint={i18n.str('blankErrorHint')}
Expand Down Expand Up @@ -49,5 +50,6 @@ PasswordPane.propTypes = {
onChange: PropTypes.func,
placeholder: PropTypes.string.isRequired,
policy: PropTypes.string,
strengthMessages: PropTypes.object
strengthMessages: PropTypes.object,
hidden: PropTypes.bool
};
3 changes: 3 additions & 0 deletions test/helper/ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ export const hasOneSocialBigButton = hasOneViewFn(
'.auth0-lock-social-button.auth0-lock-social-big-button'
);
export const hasPasswordInput = hasInputFn('password');
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this used? Should it account for the input being hidden?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe that's handled by the hasHiddenPasswordInput you added but still worth a consideration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure what you mean 😬

export const hasHiddenPasswordInput = lock =>
hasFn('.auth0-lock-input-block.auth0-lock-input-show-password.auth0-lock-hidden')(lock) &&
hasPasswordInput(lock);
export const hasTermsCheckbox = hasFn(
".auth0-lock-sign-up-terms-agreement label input[type='checkbox']"
);
Expand Down
4 changes: 2 additions & 2 deletions test/layout.ui.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ describe('layout', function() {
expect(h.hasSocialButtons(this.lock)).to.not.be.ok();
expect(h.hasEmailInput(this.lock)).to.be.ok();
expect(h.hasUsernameInput(this.lock)).to.not.be.ok();
expect(h.hasPasswordInput(this.lock)).to.not.be.ok();
expect(h.hasHiddenPasswordInput(this.lock)).to.be.ok();
expect(h.hasAlternativeLink(this.lock)).to.not.be.ok(); // forgot password
expect(h.hasSubmitButton(this.lock)).to.be.ok();
});
Expand All @@ -233,7 +233,7 @@ describe('layout', function() {
expect(h.hasSocialButtons(this.lock)).to.not.be.ok();
expect(h.hasEmailInput(this.lock)).to.be.ok();
expect(h.hasUsernameInput(this.lock)).to.not.be.ok();
expect(h.hasPasswordInput(this.lock)).to.not.be.ok();
expect(h.hasHiddenPasswordInput(this.lock)).to.be.ok();
expect(h.hasAlternativeLink(this.lock)).to.not.be.ok(); // forgot password
expect(h.hasSubmitButton(this.lock)).to.be.ok();
});
Expand Down