From 4e152f82e747b39eb9227277af4a26f20a0c527f Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 5 Feb 2019 14:30:45 +0000 Subject: [PATCH 1/5] Remove server types from login --- src/components/structures/auth/Login.js | 109 +++--------------------- 1 file changed, 14 insertions(+), 95 deletions(-) diff --git a/src/components/structures/auth/Login.js b/src/components/structures/auth/Login.js index 17da4a855f..f9964a9ed3 100644 --- a/src/components/structures/auth/Login.js +++ b/src/components/structures/auth/Login.js @@ -26,7 +26,6 @@ import Login from '../../../Login'; import SdkConfig from '../../../SdkConfig'; import { messageForResourceLimitError } from '../../../utils/ErrorUtils'; import { AutoDiscovery } from "matrix-js-sdk"; -import * as ServerType from '../../views/auth/ServerTypeSelector'; // For validating phone numbers without country codes const PHONE_NUMBER_REGEX = /^[0-9()\-\s]*$/; @@ -87,7 +86,6 @@ module.exports = React.createClass({ errorText: null, loginIncorrect: false, - serverType: null, enteredHsUrl: this.props.customHsUrl || this.props.defaultHsUrl, enteredIsUrl: this.props.customIsUrl || this.props.defaultIsUrl, @@ -263,11 +261,6 @@ module.exports = React.createClass({ username: username, discoveryError: null, }); - // If the free server type is selected, we don't show server details at all, - // so it doesn't make sense to try .well-known discovery. - if (this.state.serverType === ServerType.FREE) { - return; - } if (username[0] === "@") { const serverName = username.split(':').slice(1).join(':'); try { @@ -323,39 +316,6 @@ module.exports = React.createClass({ }); }, - onServerTypeChange(type) { - this.setState({ - serverType: type, - }); - - // When changing server types, set the HS / IS URLs to reasonable defaults for the - // the new type. - switch (type) { - case ServerType.FREE: { - const { hsUrl, isUrl } = ServerType.TYPES.FREE; - this.onServerConfigChange({ - hsUrl, - isUrl, - }); - // Move directly to the login phase since the server details are fixed. - this.setState({ - phase: PHASE_LOGIN, - }); - break; - } - case ServerType.PREMIUM: - case ServerType.ADVANCED: - this.onServerConfigChange({ - hsUrl: this.props.defaultHsUrl, - isUrl: this.props.defaultIsUrl, - }); - this.setState({ - phase: PHASE_SERVER_DETAILS, - }); - break; - } - }, - onRegisterClick: function(ev) { ev.preventDefault(); ev.stopPropagation(); @@ -391,14 +351,6 @@ module.exports = React.createClass({ try { const discovery = await AutoDiscovery.findClientConfig(serverName); - // The server type may have changed while discovery began in the background. - // If it has become the free server type which doesn't show server details, - // ignore discovery results. - if (this.state.serverType === ServerType.FREE) { - this.setState({findingHomeserver: false}); - return; - } - const state = discovery["m.homeserver"].state; if (state !== AutoDiscovery.SUCCESS && state !== AutoDiscovery.PROMPT) { this.setState({ @@ -554,51 +506,27 @@ module.exports = React.createClass({ return errorText; }, - renderServerComponentForStep() { - const ServerTypeSelector = sdk.getComponent("auth.ServerTypeSelector"); + renderServerComponent() { const ServerConfig = sdk.getComponent("auth.ServerConfig"); - const ModularServerConfig = sdk.getComponent("auth.ModularServerConfig"); const AccessibleButton = sdk.getComponent("elements.AccessibleButton"); if (SdkConfig.get()['disable_custom_urls']) { return null; } - // If we're on a different phase, we only show the server type selector, - // which is always shown if we allow custom URLs at all. if (PHASES_ENABLED && this.state.phase !== PHASE_SERVER_DETAILS) { - return
- -
; + // TODO: ... + return null; } - let serverDetails = null; - switch (this.state.serverType) { - case ServerType.FREE: - break; - case ServerType.PREMIUM: - serverDetails = ; - break; - case ServerType.ADVANCED: - serverDetails = ; - break; - } + const serverDetails = ; let nextButton = null; if (PHASES_ENABLED) { @@ -610,10 +538,6 @@ module.exports = React.createClass({ } return
- {serverDetails} {nextButton}
; @@ -642,13 +566,8 @@ module.exports = React.createClass({ _renderPasswordStep: function() { const PasswordLogin = sdk.getComponent('auth.PasswordLogin'); let onEditServerDetailsClick = null; - // If custom URLs are allowed and we haven't selected the Free server type, wire - // up the server details edit link. - if ( - PHASES_ENABLED && - !SdkConfig.get()['disable_custom_urls'] && - this.state.serverType !== ServerType.FREE - ) { + // If custom URLs are allowed, wire up the server details edit link. + if (PHASES_ENABLED && !SdkConfig.get()['disable_custom_urls']) { onEditServerDetailsClick = this.onEditServerDetailsClick; } return ( @@ -721,7 +640,7 @@ module.exports = React.createClass({ {loader} { errorTextSection } - { this.renderServerComponentForStep() } + { this.renderServerComponent() } { this.renderLoginComponentForStep() } { _t('Create account') } From dbe14c5527c62d5dcd7bfacdb965e5d5e03d362d Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 5 Feb 2019 14:43:25 +0000 Subject: [PATCH 2/5] Enable phases for login, but start on the login phase by default --- src/components/structures/auth/Login.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/structures/auth/Login.js b/src/components/structures/auth/Login.js index f9964a9ed3..d645ade967 100644 --- a/src/components/structures/auth/Login.js +++ b/src/components/structures/auth/Login.js @@ -31,13 +31,13 @@ import { AutoDiscovery } from "matrix-js-sdk"; const PHONE_NUMBER_REGEX = /^[0-9()\-\s]*$/; // Phases -// Show controls to configure server details -const PHASE_SERVER_DETAILS = 0; // Show the appropriate login flow(s) for the server -const PHASE_LOGIN = 1; +const PHASE_LOGIN = 0; +// Show controls to configure server details +const PHASE_SERVER_DETAILS = 1; -// Disable phases for now, pending UX discussion on WK discovery -const PHASES_ENABLED = false; +// Enable phases for login +const PHASES_ENABLED = true; // These are used in several places, and come from the js-sdk's autodiscovery // stuff. We define them here so that they'll be picked up by i18n. @@ -95,7 +95,7 @@ module.exports = React.createClass({ phoneNumber: "", // Phase of the overall login dialog. - phase: PHASE_SERVER_DETAILS, + phase: PHASE_LOGIN, // The current login flow, such as password, SSO, etc. currentFlow: "m.login.password", From 02a20fa6684ec9c9d0f93f79692052a17ceab412 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 5 Feb 2019 15:36:00 +0000 Subject: [PATCH 3/5] Tweak login wording to match design --- src/components/structures/auth/Login.js | 2 +- src/components/views/auth/PasswordLogin.js | 8 ++++---- src/i18n/strings/en_EN.json | 10 ++++------ 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/components/structures/auth/Login.js b/src/components/structures/auth/Login.js index d645ade967..c6de095461 100644 --- a/src/components/structures/auth/Login.js +++ b/src/components/structures/auth/Login.js @@ -636,7 +636,7 @@ module.exports = React.createClass({

- {_t('Sign in to your account')} + {_t('Sign in')} {loader}

{ errorTextSection } diff --git a/src/components/views/auth/PasswordLogin.js b/src/components/views/auth/PasswordLogin.js index 5bc6d6e05b..8db30c6d43 100644 --- a/src/components/views/auth/PasswordLogin.js +++ b/src/components/views/auth/PasswordLogin.js @@ -249,10 +249,10 @@ class PasswordLogin extends React.Component { ; } - let yourMatrixAccountText = _t('Your account'); + let signInToText = _t('Sign in'); try { const parsedHsUrl = new URL(this.props.hsUrl); - yourMatrixAccountText = _t('Your %(serverName)s account', { + signInToText = _t('Sign in to %(serverName)s', { serverName: parsedHsUrl.hostname, }); } catch (e) { @@ -264,7 +264,7 @@ class PasswordLogin extends React.Component { editLink = - {_t('Edit')} + {_t('Change')} ; } @@ -297,7 +297,7 @@ class PasswordLogin extends React.Component { return (

- {yourMatrixAccountText} + {signInToText} {editLink}

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 623dd92613..9e5b32f24c 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -870,12 +870,12 @@ "Delete widget": "Delete widget", "Failed to remove widget": "Failed to remove widget", "An error ocurred whilst trying to remove the widget from the room": "An error ocurred whilst trying to remove the widget from the room", - "Revoke widget access": "Revoke widget access", "Minimize apps": "Minimize apps", "Reload widget": "Reload widget", "Popout widget": "Popout widget", "Picture": "Picture", "Edit": "Edit", + "Revoke widget access": "Revoke widget access", "Create new room": "Create new room", "Unblacklist": "Unblacklist", "Blacklist": "Blacklist", @@ -1064,7 +1064,6 @@ "To help avoid duplicate issues, please view existing issues first (and add a +1) or create a new issue if you can't find it.": "To help avoid duplicate issues, please view existing issues first (and add a +1) or create a new issue if you can't find it.", "Report bugs & give feedback": "Report bugs & give feedback", "Go back": "Go back", - "Visit old settings": "Visit old settings", "Room Settings": "Room Settings", "Failed to upgrade room": "Failed to upgrade room", "The room upgrade could not be completed": "The room upgrade could not be completed", @@ -1196,10 +1195,10 @@ "Username": "Username", "Mobile phone number": "Mobile phone number", "Not sure of your password? Set a new one": "Not sure of your password? Set a new one", - "Your account": "Your account", - "Your %(serverName)s account": "Your %(serverName)s account", - "Sign in with": "Sign in with", "Sign in": "Sign in", + "Sign in to %(serverName)s": "Sign in to %(serverName)s", + "Change": "Change", + "Sign in with": "Sign in with", "If you don't specify an email address, you won't be able to reset your password. Are you sure?": "If you don't specify an email address, you won't be able to reset your password. Are you sure?", "Create your account": "Create your account", "Create your %(serverName)s account": "Create your %(serverName)s account", @@ -1403,7 +1402,6 @@ "Can't connect to homeserver - please check your connectivity, ensure your homeserver's SSL certificate is trusted, and that a browser extension is not blocking requests.": "Can't connect to homeserver - please check your connectivity, ensure your homeserver's SSL certificate is trusted, and that a browser extension is not blocking requests.", "Sign in with single sign-on": "Sign in with single sign-on", "Try the app first": "Try the app first", - "Sign in to your account": "Sign in to your account", "Create account": "Create account", "Failed to fetch avatar URL": "Failed to fetch avatar URL", "Set a display name:": "Set a display name:", From 6549ae1ce54b766ab2a657c4b3f8ad2559a8e80b Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 5 Feb 2019 16:11:32 +0000 Subject: [PATCH 4/5] Remove unused props from login --- src/components/structures/MatrixChat.js | 6 ------ src/components/structures/auth/Login.js | 1 - 2 files changed, 7 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 77800109e4..24db0dc995 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -1676,11 +1676,6 @@ export default React.createClass({ this.showScreen("forgot_password"); }, - onReturnToAppClick: function() { - // treat it the same as if the user had completed the login - this._onLoggedIn(); - }, - // returns a promise which resolves to the new MatrixClient onRegistered: function(credentials) { // XXX: This should be in state or ideally store(s) because we risk not @@ -1936,7 +1931,6 @@ export default React.createClass({ defaultDeviceDisplayName={this.props.defaultDeviceDisplayName} onForgotPasswordClick={this.onForgotPasswordClick} enableGuest={this.props.enableGuest} - onCancelClick={MatrixClientPeg.get() ? this.onReturnToAppClick : null} onServerConfigChange={this.onServerConfigChange} /> ); diff --git a/src/components/structures/auth/Login.js b/src/components/structures/auth/Login.js index c6de095461..8524447ed4 100644 --- a/src/components/structures/auth/Login.js +++ b/src/components/structures/auth/Login.js @@ -76,7 +76,6 @@ module.exports = React.createClass({ // login shouldn't care how password recovery is done. onForgotPasswordClick: PropTypes.func, - onCancelClick: PropTypes.func, onServerConfigChange: PropTypes.func.isRequired, }, From 71c30b564181d71083d5202043067cc9a536ca3f Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 5 Feb 2019 16:33:12 +0000 Subject: [PATCH 5/5] Add some basic login tests --- test/components/structures/auth/Login-test.js | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 test/components/structures/auth/Login-test.js diff --git a/test/components/structures/auth/Login-test.js b/test/components/structures/auth/Login-test.js new file mode 100644 index 0000000000..ec95243a56 --- /dev/null +++ b/test/components/structures/auth/Login-test.js @@ -0,0 +1,90 @@ +/* +Copyright 2019 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import expect from 'expect'; +import sinon from 'sinon'; +import React from 'react'; +import ReactDOM from 'react-dom'; +import ReactTestUtils from 'react-dom/test-utils'; +import sdk from 'matrix-react-sdk'; +import SdkConfig from '../../../../src/SdkConfig'; +import * as TestUtils from '../../../test-utils'; + +const Login = sdk.getComponent( + 'structures.auth.Login', +); + +describe('Login', function() { + let parentDiv; + + beforeEach(function() { + TestUtils.beforeEach(this); + parentDiv = document.createElement('div'); + document.body.appendChild(parentDiv); + }); + + afterEach(function() { + sinon.restore(); + ReactDOM.unmountComponentAtNode(parentDiv); + parentDiv.remove(); + }); + + function render() { + return ReactDOM.render( {}} + onRegisterClick={() => {}} + onServerConfigChange={() => {}} + />, parentDiv); + } + + it('should show form with change server link', function() { + const root = render(); + + const form = ReactTestUtils.findRenderedComponentWithType( + root, + sdk.getComponent('auth.PasswordLogin'), + ); + expect(form).toBeTruthy(); + + const changeServerLink = ReactTestUtils.findRenderedDOMComponentWithClass( + root, + 'mx_AuthBody_editServerDetails', + ); + expect(changeServerLink).toBeTruthy(); + }); + + it('should show form without change server link when custom URLs disabled', function() { + sinon.stub(SdkConfig, "get").returns({ + disable_custom_urls: true, + }); + + const root = render(); + + const form = ReactTestUtils.findRenderedComponentWithType( + root, + sdk.getComponent('auth.PasswordLogin'), + ); + expect(form).toBeTruthy(); + + const changeServerLinks = ReactTestUtils.scryRenderedDOMComponentsWithClass( + root, + 'mx_AuthBody_editServerDetails', + ); + expect(changeServerLinks).toHaveLength(0); + }); +});