From 40f16fa310d85053235e3c79ea950e559d6a33da Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Fri, 1 Feb 2019 00:36:19 +0100 Subject: [PATCH 01/11] adds validation for fields. * renames RoomTooltip to be a generic Tooltip (which it is) * hooks it into Field to show validation results * adds onValidate to Field to let Field instances call an arbitrary validation function Rebased from @ara4n's https://github.com/matrix-org/matrix-react-sdk/pull/2550 by @jryans. Subsequent commits revise and adapt this work. --- res/css/_components.scss | 2 +- res/css/views/elements/_Field.scss | 33 ++++++++++++ .../_Tooltip.scss} | 53 ++++++++++++------- res/themes/dark/css/_dark.scss | 1 + res/themes/light/css/_light.scss | 3 +- src/components/structures/BottomLeftMenu.js | 4 +- src/components/views/auth/ServerConfig.js | 10 ++++ src/components/views/elements/ActionButton.js | 4 +- src/components/views/elements/Field.js | 53 +++++++++++++++++-- src/components/views/elements/TagTile.js | 4 +- .../views/elements/ToolTipButton.js | 4 +- .../RoomTooltip.js => elements/Tooltip.js} | 14 ++--- .../views/groups/GroupInviteTile.js | 4 +- src/components/views/messages/MStickerBody.js | 4 +- src/components/views/rooms/RoomTile.js | 4 +- 15 files changed, 152 insertions(+), 45 deletions(-) rename res/css/views/{rooms/_RoomTooltip.scss => elements/_Tooltip.scss} (54%) rename src/components/views/{rooms/RoomTooltip.js => elements/Tooltip.js} (89%) diff --git a/res/css/_components.scss b/res/css/_components.scss index 6f66a8c15e0..4fb0eed4afc 100644 --- a/res/css/_components.scss +++ b/res/css/_components.scss @@ -95,6 +95,7 @@ @import "./views/elements/_SyntaxHighlight.scss"; @import "./views/elements/_ToggleSwitch.scss"; @import "./views/elements/_ToolTipButton.scss"; +@import "./views/elements/_Tooltip.scss"; @import "./views/globals/_MatrixToolbar.scss"; @import "./views/groups/_GroupPublicityToggle.scss"; @import "./views/groups/_GroupRoomList.scss"; @@ -137,7 +138,6 @@ @import "./views/rooms/_RoomPreviewBar.scss"; @import "./views/rooms/_RoomRecoveryReminder.scss"; @import "./views/rooms/_RoomTile.scss"; -@import "./views/rooms/_RoomTooltip.scss"; @import "./views/rooms/_RoomUpgradeWarningBar.scss"; @import "./views/rooms/_SearchBar.scss"; @import "./views/rooms/_SearchableEntityList.scss"; diff --git a/res/css/views/elements/_Field.scss b/res/css/views/elements/_Field.scss index 4a74262fd4f..22bc6a1800b 100644 --- a/res/css/views/elements/_Field.scss +++ b/res/css/views/elements/_Field.scss @@ -141,6 +141,39 @@ limitations under the License. color: $greyed-fg-color; } +.mx_Field_valid input, +.mx_Field_valid select, +.mx_Field_valid textarea { + border-color: $input-valid-border-color ! important; +} + +.mx_Field_valid input + label, +.mx_Field_valid select + label, +.mx_Field_valid textarea + label { + color: $input-valid-border-color ! important; +} + +.mx_Field_invalid input, +.mx_Field_invalid select, +.mx_Field_invalid textarea { + border-color: $input-invalid-border-color ! important; +} + +.mx_Field_invalid input + label, +.mx_Field_invalid select + label, +.mx_Field_invalid textarea + label { + color: $input-invalid-border-color ! important; +} + +.mx_Field_tooltip { + margin-top: -12px; + margin-left: 4px; +} + +.mx_Field_tooltip.mx_Field_valid { + animation: mx_fadeout 1s 2s forwards; +} + // Customise other components when placed inside a Field .mx_Field .mx_Dropdown_input { diff --git a/res/css/views/rooms/_RoomTooltip.scss b/res/css/views/elements/_Tooltip.scss similarity index 54% rename from res/css/views/rooms/_RoomTooltip.scss rename to res/css/views/elements/_Tooltip.scss index 295786d2d3c..78604b15645 100644 --- a/res/css/views/rooms/_RoomTooltip.scss +++ b/res/css/views/elements/_Tooltip.scss @@ -1,5 +1,6 @@ /* Copyright 2015, 2016 OpenMarket Ltd +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. @@ -14,41 +15,55 @@ See the License for the specific language governing permissions and limitations under the License. */ -.mx_RoomTooltip_chevron { +@keyframes mx_fadein { + from { opacity: 0; } + to { opacity: 1; } +} + +@keyframes mx_fadeout { + from { opacity: 1; } + to { opacity: 0; } +} + +.mx_Tooltip_chevron { position: absolute; - left: -8px; - top: 4px; + left: -7px; + top: 10px; width: 0; height: 0; - border-top: 8px solid transparent; - border-right: 8px solid $menu-bg-color; - border-bottom: 8px solid transparent; + border-top: 7px solid transparent; + border-right: 7px solid $menu-border-color; + border-bottom: 7px solid transparent; } -.mx_RoomTooltip_chevron:after { +.mx_Tooltip_chevron:after { content:''; width: 0; height: 0; - border-top: 7px solid transparent; - border-right: 7px solid $primary-bg-color; - border-bottom: 7px solid transparent; - position:absolute; - top: -7px; + border-top: 6px solid transparent; + border-right: 6px solid $menu-bg-color; + border-bottom: 6px solid transparent; + position: absolute; + top: -6px; left: 1px; } -.mx_RoomTooltip { +.mx_Tooltip { display: none; + animation: mx_fadein 0.2s; position: fixed; - border-radius: 5px; - box-shadow: 0 0 5px 0 rgba(0, 0, 0, 0.21); - background-color: $primary-bg-color; + border: 1px solid $menu-border-color; + border-radius: 4px; + box-shadow: 4px 4px 12px 0 rgba(118, 131, 156, 0.6); + background-color: $menu-bg-color; z-index: 2000; - padding: 5px; + padding: 10px; pointer-events: none; line-height: 14px; - font-size: 13px; + font-size: 12px; + font-weight: 600; color: $primary-fg-color; - max-width: 600px; + max-width: 200px; + word-break: break-word; margin-right: 50px; } diff --git a/res/themes/dark/css/_dark.scss b/res/themes/dark/css/_dark.scss index deed7235cb6..3112644a73d 100644 --- a/res/themes/dark/css/_dark.scss +++ b/res/themes/dark/css/_dark.scss @@ -53,6 +53,7 @@ $input-lighter-bg-color: #f2f5f8; $input-lighter-fg-color: $input-darker-fg-color; $input-focused-border-color: #238cf5; $input-valid-border-color: $accent-color; +$input-invalid-border-color: $warning-color; $field-focused-label-bg-color: $bg-color; diff --git a/res/themes/light/css/_light.scss b/res/themes/light/css/_light.scss index 4d8e4fa40ea..879be67ddae 100644 --- a/res/themes/light/css/_light.scss +++ b/res/themes/light/css/_light.scss @@ -79,6 +79,7 @@ $input-lighter-bg-color: #f2f5f8; $input-lighter-fg-color: $input-darker-fg-color; $input-focused-border-color: #238cf5; $input-valid-border-color: $accent-color; +$input-invalid-border-color: $warning-color; $field-focused-label-bg-color: #ffffff; @@ -95,7 +96,7 @@ $input-fg-color: rgba(74, 74, 74, 0.9); $scrollbar-thumb-color: rgba(0, 0, 0, 0.2); $scrollbar-track-color: transparent; // context menus -$menu-border-color: #ebedf8; +$menu-border-color: #e7e7e7; $menu-bg-color: #fff; $menu-box-shadow-color: rgba(118, 131, 156, 0.6); $menu-selected-color: #f5f8fa; diff --git a/src/components/structures/BottomLeftMenu.js b/src/components/structures/BottomLeftMenu.js index 47b30be450b..2f48bd0299c 100644 --- a/src/components/structures/BottomLeftMenu.js +++ b/src/components/structures/BottomLeftMenu.js @@ -145,8 +145,8 @@ module.exports = React.createClass({ // Get the label/tooltip to show getLabel: function(label, show) { if (show) { - const RoomTooltip = sdk.getComponent("rooms.RoomTooltip"); - return ; + const Tooltip = sdk.getComponent("elements.Tooltip"); + return ; } }, diff --git a/src/components/views/auth/ServerConfig.js b/src/components/views/auth/ServerConfig.js index cb0e0dc38ec..ed6b4bdf7dc 100644 --- a/src/components/views/auth/ServerConfig.js +++ b/src/components/views/auth/ServerConfig.js @@ -90,6 +90,15 @@ export default class ServerConfig extends React.PureComponent { this.setState({ hsUrl }); } + onHomeserverValidate = (value) => { + try { + new URL(value); + return { valid: true, feedback:
Valid URL!
}; + } catch (_) { + return { valid: false, feedback:
Invalid URL!
}; + } + } + onIdentityServerBlur = (ev) => { this._isTimeoutId = this._waitThenInvoke(this._isTimeoutId, () => { this.props.onServerConfigChange({ @@ -134,6 +143,7 @@ export default class ServerConfig extends React.PureComponent { value={this.state.hsUrl} onBlur={this.onHomeserverBlur} onChange={this.onHomeserverChange} + onValidate={this.onHomeserverValidate} /> ; + const Tooltip = sdk.getComponent("elements.Tooltip"); + tooltip = ; } const icon = this.props.iconPath ? diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index c6a2113adb0..eb3fcf272dd 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -17,6 +17,7 @@ limitations under the License. import React from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; +import sdk from '../../../index'; export default class Field extends React.PureComponent { static propTypes = { @@ -33,9 +34,22 @@ export default class Field extends React.PureComponent { placeholder: PropTypes.string, // Optional component to include inside the field before the input. prefix: PropTypes.node, + // The callback called whenever the contents of the field + // changes. Returns an object with `valid` boolean field + // and a `feedback` react component field to provide feedback + // to the user. + onValidate: PropTypes.function, // All other props pass through to the . }; + constructor() { + super(); + this.state = { + valid: undefined, + feedback: undefined, + }; + } + get value() { if (!this.refs.fieldInput) return null; return this.refs.fieldInput.value; @@ -48,8 +62,18 @@ export default class Field extends React.PureComponent { this.refs.fieldInput.value = newValue; } + onChange = (ev) => { + if (this.props.onValidate) { + const result = this.props.onValidate(this.value); + this.setState({ + valid: result.valid, + feedback: result.feedback, + }); + } + }; + render() { - const { element, prefix, children, ...inputProps } = this.props; + const { element, prefix, onValidate, children, ...inputProps } = this.props; const inputElement = element || "input"; @@ -58,6 +82,12 @@ export default class Field extends React.PureComponent { inputProps.ref = "fieldInput"; inputProps.placeholder = inputProps.placeholder || inputProps.label; + inputProps.onChange = this.onChange; + // make sure we use the current `value` for the field and not the original one + if (this.value != undefined) { + inputProps.value = this.value; + } + const fieldInput = React.createElement(inputElement, inputProps, children); let prefixContainer = null; @@ -65,17 +95,34 @@ export default class Field extends React.PureComponent { prefixContainer = {prefix}; } - const classes = classNames("mx_Field", `mx_Field_${inputElement}`, { + const validClass = classNames({ + mx_Field_valid: this.state.valid === true, + mx_Field_invalid: this.state.valid === false, + }); + + const fieldClasses = classNames("mx_Field", `mx_Field_${inputElement}`, { // If we have a prefix element, leave the label always at the top left and // don't animate it, as it looks a bit clunky and would add complexity to do // properly. mx_Field_labelAlwaysTopLeft: prefix, + [validClass]: true, }); - return
+ // handle displaying feedback on validity + const Tooltip = sdk.getComponent("elements.Tooltip"); + let feedback; + if (this.state.feedback) { + feedback = ; + } + + return
{prefixContainer} {fieldInput} + {feedback}
; } } diff --git a/src/components/views/elements/TagTile.js b/src/components/views/elements/TagTile.js index f5ee60a2d80..ef9864358b5 100644 --- a/src/components/views/elements/TagTile.js +++ b/src/components/views/elements/TagTile.js @@ -156,7 +156,7 @@ export default React.createClass({ render: function() { const BaseAvatar = sdk.getComponent('avatars.BaseAvatar'); const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); - const RoomTooltip = sdk.getComponent('rooms.RoomTooltip'); + const Tooltip = sdk.getComponent('elements.Tooltip'); const profile = this.state.profile || {}; const name = profile.name || this.props.tag; const avatarHeight = 40; @@ -181,7 +181,7 @@ export default React.createClass({ } const tip = this.state.hover ? - : + :
; const contextButton = this.state.hover || this.state.menuDisplayed ?
diff --git a/src/components/views/elements/ToolTipButton.js b/src/components/views/elements/ToolTipButton.js index b5b2d735ee5..239095f1963 100644 --- a/src/components/views/elements/ToolTipButton.js +++ b/src/components/views/elements/ToolTipButton.js @@ -39,8 +39,8 @@ module.exports = React.createClass({ }, render: function() { - const RoomTooltip = sdk.getComponent("rooms.RoomTooltip"); - const tip = this.state.hover ? -
+
{ this.props.label }
); diff --git a/src/components/views/groups/GroupInviteTile.js b/src/components/views/groups/GroupInviteTile.js index 44441f47543..8482bce5936 100644 --- a/src/components/views/groups/GroupInviteTile.js +++ b/src/components/views/groups/GroupInviteTile.js @@ -143,8 +143,8 @@ export default React.createClass({ let tooltip; if (this.props.collapsed && this.state.hover) { - const RoomTooltip = sdk.getComponent("rooms.RoomTooltip"); - tooltip = ; + const Tooltip = sdk.getComponent("elements.Tooltip"); + tooltip = ; } const classes = classNames('mx_RoomTile mx_RoomTile_highlight', { diff --git a/src/components/views/messages/MStickerBody.js b/src/components/views/messages/MStickerBody.js index 55263ef7b71..6a4128dfa7d 100644 --- a/src/components/views/messages/MStickerBody.js +++ b/src/components/views/messages/MStickerBody.js @@ -44,9 +44,9 @@ export default class MStickerBody extends MImageBody { if (!content || !content.body || !content.info || !content.info.w) return null; - const RoomTooltip = sdk.getComponent('rooms.RoomTooltip'); + const Tooltip = sdk.getComponent('elements.Tooltip'); return
- +
; } diff --git a/src/components/views/rooms/RoomTile.js b/src/components/views/rooms/RoomTile.js index f9e9d64b9e3..4bf160007e3 100644 --- a/src/components/views/rooms/RoomTile.js +++ b/src/components/views/rooms/RoomTile.js @@ -364,8 +364,8 @@ module.exports = React.createClass({ label = { name }; } } else if (this.state.hover) { - const RoomTooltip = sdk.getComponent("rooms.RoomTooltip"); - tooltip = ; + const Tooltip = sdk.getComponent("elements.Tooltip"); + tooltip = ; } //var incomingCallBox; From edb7f39ec97360afde247d3f90abfbac57d0b5e7 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 7 Mar 2019 13:24:23 +0000 Subject: [PATCH 02/11] Validity class currently unused on tooltip --- src/components/views/elements/Field.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index eb3fcf272dd..8a203c36675 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -113,7 +113,7 @@ export default class Field extends React.PureComponent { let feedback; if (this.state.feedback) { feedback = ; } From b8925d857d5f50d065ba6b3b379a2f3fd826c141 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 7 Mar 2019 14:16:39 +0000 Subject: [PATCH 03/11] Reorganize field validity styles * The field border style was previously moved up to the field * Validity colors should be shown regardless of focus state --- res/css/views/elements/_Field.scss | 44 +++++++++++++++--------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/res/css/views/elements/_Field.scss b/res/css/views/elements/_Field.scss index 22bc6a1800b..20b1efd28bf 100644 --- a/res/css/views/elements/_Field.scss +++ b/res/css/views/elements/_Field.scss @@ -141,28 +141,28 @@ limitations under the License. color: $greyed-fg-color; } -.mx_Field_valid input, -.mx_Field_valid select, -.mx_Field_valid textarea { - border-color: $input-valid-border-color ! important; -} - -.mx_Field_valid input + label, -.mx_Field_valid select + label, -.mx_Field_valid textarea + label { - color: $input-valid-border-color ! important; -} - -.mx_Field_invalid input, -.mx_Field_invalid select, -.mx_Field_invalid textarea { - border-color: $input-invalid-border-color ! important; -} - -.mx_Field_invalid input + label, -.mx_Field_invalid select + label, -.mx_Field_invalid textarea + label { - color: $input-invalid-border-color ! important; +.mx_Field_valid { + &.mx_Field, + &.mx_Field:focus-within { + border-color: $input-valid-border-color; + } + + &.mx_Field label, + &.mx_Field:focus-within label { + color: $input-valid-border-color; + } +} + +.mx_Field_invalid { + &.mx_Field, + &.mx_Field:focus-within { + border-color: $input-invalid-border-color; + } + + &.mx_Field label, + &.mx_Field:focus-within label { + color: $input-invalid-border-color; + } } .mx_Field_tooltip { From 7241418ebafbbd58749de92e54e35421b823ebb6 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 7 Mar 2019 14:34:55 +0000 Subject: [PATCH 04/11] Appease the linter This checks `onValidate` in `render` to make the linter happy. --- src/components/views/elements/Field.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index 8a203c36675..b710872d2d1 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -95,10 +95,13 @@ export default class Field extends React.PureComponent { prefixContainer = {prefix}; } - const validClass = classNames({ - mx_Field_valid: this.state.valid === true, - mx_Field_invalid: this.state.valid === false, - }); + let validClass; + if (onValidate) { + validClass = classNames({ + mx_Field_valid: this.state.valid === true, + mx_Field_invalid: this.state.valid === false, + }); + } const fieldClasses = classNames("mx_Field", `mx_Field_${inputElement}`, { // If we have a prefix element, leave the label always at the top left and From 57ce4d6e7d19c30bba394edb4fdf866883d76a2e Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 7 Mar 2019 14:45:18 +0000 Subject: [PATCH 05/11] Call the parent component's `onChange` if it exists --- src/components/views/elements/Field.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index b710872d2d1..4ccb3e94409 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -70,6 +70,10 @@ export default class Field extends React.PureComponent { feedback: result.feedback, }); } + // Parent component may have supplied its own `onChange` as well + if (this.props.onChange) { + this.props.onChange(ev); + } }; render() { From ea050683bdebe7acd6d9e86bf63535655af19521 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 7 Mar 2019 15:39:42 +0000 Subject: [PATCH 06/11] Use the right prop type for functions --- src/components/views/elements/Field.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index 4ccb3e94409..5bb4067ae21 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -38,7 +38,7 @@ export default class Field extends React.PureComponent { // changes. Returns an object with `valid` boolean field // and a `feedback` react component field to provide feedback // to the user. - onValidate: PropTypes.function, + onValidate: PropTypes.func, // All other props pass through to the . }; From 5a648ecfe453159a02c44f85193ad256475d5d0e Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 7 Mar 2019 16:41:44 +0000 Subject: [PATCH 07/11] Ensure we always set some value in Field Always set some value on the Field's input so that it doesn't flip flop between controlled and uncontrolled. --- src/components/views/elements/Field.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index 5bb4067ae21..36f6f02ff24 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -88,8 +88,8 @@ export default class Field extends React.PureComponent { inputProps.onChange = this.onChange; // make sure we use the current `value` for the field and not the original one - if (this.value != undefined) { - inputProps.value = this.value; + if (inputProps.value === undefined) { + inputProps.value = this.value || ""; } const fieldInput = React.createElement(inputElement, inputProps, children); From cff3c948586c066bb3fdcc7d0288c968d74c41a3 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 12 Mar 2019 15:39:30 +0000 Subject: [PATCH 08/11] Fix indentation in PhoneNumbers.js --- src/components/views/settings/PhoneNumbers.js | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/components/views/settings/PhoneNumbers.js b/src/components/views/settings/PhoneNumbers.js index d85642d3468..b8092409566 100644 --- a/src/components/views/settings/PhoneNumbers.js +++ b/src/components/views/settings/PhoneNumbers.js @@ -205,22 +205,22 @@ export default class PhoneNumbers extends React.Component { if (this.state.verifying) { const msisdn = this.state.verifyMsisdn; addVerifySection = ( -
-
- {_t("A text message has been sent to +%(msisdn)s. " + - "Please enter the verification code it contains", {msisdn: msisdn})} -
- {this.state.verifyError} -
-
- - - {_t("Continue")} - - -
+
+
+ {_t("A text message has been sent to +%(msisdn)s. " + + "Please enter the verification code it contains", { msisdn: msisdn })} +
+ {this.state.verifyError} +
+
+ + + {_t("Continue")} + + +
); } From d4dbba3938f73c49edca403a3e4375b1fe58a231 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 12 Mar 2019 16:17:21 +0000 Subject: [PATCH 09/11] Convert uncontrolled Field usages to controlled As part of adding validation to Field, the logic is simpler to follow if we can assume that all usages of Field use it as a controlled component, instead of supporting both controlled and uncontrolled. This converts the uncontrolled usages to controlled. --- src/components/views/auth/PasswordLogin.js | 9 ---- src/components/views/elements/Field.js | 6 +++ .../views/settings/ChangePassword.js | 54 ++++++++++++++++--- .../views/settings/EmailAddresses.js | 25 ++++++--- src/components/views/settings/PhoneNumbers.js | 45 ++++++++++++---- 5 files changed, 108 insertions(+), 31 deletions(-) diff --git a/src/components/views/auth/PasswordLogin.js b/src/components/views/auth/PasswordLogin.js index 4b095d405f7..ed3afede2fc 100644 --- a/src/components/views/auth/PasswordLogin.js +++ b/src/components/views/auth/PasswordLogin.js @@ -70,11 +70,6 @@ class PasswordLogin extends React.Component { this.isLoginEmpty = this.isLoginEmpty.bind(this); } - componentWillMount() { - this._passwordField = null; - this._loginField = null; - } - onForgotPasswordClick(ev) { ev.preventDefault(); ev.stopPropagation(); @@ -180,7 +175,6 @@ class PasswordLogin extends React.Component { return { this._loginField = e; }} name="username" // make it a little easier for browser's remember-password key="email_input" type="text" @@ -196,7 +190,6 @@ class PasswordLogin extends React.Component { return { this._loginField = e; }} name="username" // make it a little easier for browser's remember-password key="username_input" type="text" @@ -223,7 +216,6 @@ class PasswordLogin extends React.Component { return { this._loginField = e; }} name="phoneNumber" key="phone_input" type="text" @@ -344,7 +336,6 @@ class PasswordLogin extends React.Component { { this._passwordField = e; }} type="password" name="password" label={_t('Password')} diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index 36f6f02ff24..cd06610c62c 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -32,6 +32,9 @@ export default class Field extends React.PureComponent { label: PropTypes.string, // The field's placeholder string. Defaults to the label. placeholder: PropTypes.string, + // The field's value. + // This is a controlled component, so the value is required. + value: PropTypes.string.isRequired, // Optional component to include inside the field before the input. prefix: PropTypes.node, // The callback called whenever the contents of the field @@ -50,6 +53,7 @@ export default class Field extends React.PureComponent { }; } + /* TODO: Remove me */ get value() { if (!this.refs.fieldInput) return null; return this.refs.fieldInput.value; @@ -87,6 +91,8 @@ export default class Field extends React.PureComponent { inputProps.placeholder = inputProps.placeholder || inputProps.label; inputProps.onChange = this.onChange; + + /* TODO: Remove me */ // make sure we use the current `value` for the field and not the original one if (inputProps.value === undefined) { inputProps.value = this.value || ""; diff --git a/src/components/views/settings/ChangePassword.js b/src/components/views/settings/ChangePassword.js index 69b80b03b36..ba708beaf44 100644 --- a/src/components/views/settings/ChangePassword.js +++ b/src/components/views/settings/ChangePassword.js @@ -32,6 +32,7 @@ import sessionStore from '../../../stores/SessionStore'; module.exports = React.createClass({ displayName: 'ChangePassword', + propTypes: { onFinished: PropTypes.func, onError: PropTypes.func, @@ -73,6 +74,9 @@ module.exports = React.createClass({ return { phase: this.Phases.Edit, cachedPassword: null, + oldPassword: "", + newPassword: "", + newPasswordConfirm: "", }; }, @@ -165,6 +169,9 @@ module.exports = React.createClass({ }).finally(() => { this.setState({ phase: this.Phases.Edit, + oldPassword: "", + newPassword: "", + newPasswordConfirm: "", }); }).done(); }, @@ -192,11 +199,29 @@ module.exports = React.createClass({ ); }, + onChangeOldPassword(ev) { + this.setState({ + oldPassword: ev.target.value, + }); + }, + + onChangeNewPassword(ev) { + this.setState({ + newPassword: ev.target.value, + }); + }, + + onChangeNewPasswordConfirm(ev) { + this.setState({ + newPasswordConfirm: ev.target.value, + }); + }, + onClickChange: function(ev) { ev.preventDefault(); - const oldPassword = this.state.cachedPassword || this.refs.old_input.value; - const newPassword = this.refs.new_input.value; - const confirmPassword = this.refs.confirm_input.value; + const oldPassword = this.state.cachedPassword || this.state.oldPassword; + const newPassword = this.state.newPassword; + const confirmPassword = this.state.newPasswordConfirm; const err = this.props.onCheckPassword( oldPassword, newPassword, confirmPassword, ); @@ -217,7 +242,12 @@ module.exports = React.createClass({ if (!this.state.cachedPassword) { currentPassword = (
- +
); } @@ -230,11 +260,21 @@ module.exports = React.createClass({
{ currentPassword }
- +
- +
{ _t('Change Password') } diff --git a/src/components/views/settings/EmailAddresses.js b/src/components/views/settings/EmailAddresses.js index 1ded71a5c71..b221f919019 100644 --- a/src/components/views/settings/EmailAddresses.js +++ b/src/components/views/settings/EmailAddresses.js @@ -119,6 +119,7 @@ export default class EmailAddresses extends React.Component { verifying: false, addTask: null, continueDisabled: false, + newEmailAddress: "", }; } @@ -134,14 +135,20 @@ export default class EmailAddresses extends React.Component { this.setState({emails: this.state.emails.filter((e) => e !== address)}); }; + _onChangeNewEmailAddress = (e) => { + this.setState({ + newEmailAddress: e.target.value, + }); + }; + _onAddClick = (e) => { e.stopPropagation(); e.preventDefault(); - if (!this.refs.newEmailAddress) return; + if (!this.state.newEmailAddress) return; const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); - const email = this.refs.newEmailAddress.value; + const email = this.state.newEmailAddress; // TODO: Inline field validation if (!Email.looksValid(email)) { @@ -173,14 +180,14 @@ export default class EmailAddresses extends React.Component { this.setState({continueDisabled: true}); this.state.addTask.checkEmailLinkClicked().then(() => { - const email = this.refs.newEmailAddress.value; + const email = this.state.newEmailAddress; this.setState({ emails: [...this.state.emails, {address: email, medium: "email"}], addTask: null, continueDisabled: false, verifying: false, + newEmailAddress: "", }); - this.refs.newEmailAddress.value = ""; }).catch((err) => { this.setState({continueDisabled: false}); if (err.errcode !== 'M_THREEPID_AUTH_FAILED') { @@ -221,8 +228,14 @@ export default class EmailAddresses extends React.Component { {existingEmailElements} - + {addButton}
diff --git a/src/components/views/settings/PhoneNumbers.js b/src/components/views/settings/PhoneNumbers.js index b8092409566..5d1d9b849f3 100644 --- a/src/components/views/settings/PhoneNumbers.js +++ b/src/components/views/settings/PhoneNumbers.js @@ -117,6 +117,8 @@ export default class PhoneNumbers extends React.Component { addTask: null, continueDisabled: false, phoneCountry: "", + newPhoneNumber: "", + newPhoneNumberCode: "", }; } @@ -132,14 +134,26 @@ export default class PhoneNumbers extends React.Component { this.setState({msisdns: this.state.msisdns.filter((e) => e !== address)}); }; + _onChangeNewPhoneNumber = (e) => { + this.setState({ + newPhoneNumber: e.target.value, + }); + }; + + _onChangeNewPhoneNumberCode = (e) => { + this.setState({ + newPhoneNumberCode: e.target.value, + }); + }; + _onAddClick = (e) => { e.stopPropagation(); e.preventDefault(); - if (!this.refs.newPhoneNumber) return; + if (!this.state.newPhoneNumber) return; const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); - const phoneNumber = this.refs.newPhoneNumber.value; + const phoneNumber = this.state.newPhoneNumber; const phoneCountry = this.state.phoneCountry; const task = new AddThreepid(); @@ -162,7 +176,7 @@ export default class PhoneNumbers extends React.Component { e.preventDefault(); this.setState({continueDisabled: true}); - const token = this.refs.newPhoneNumberCode.value; + const token = this.state.newPhoneNumberCode; this.state.addTask.haveMsisdnToken(token).then(() => { this.setState({ msisdns: [...this.state.msisdns, {address: this.state.verifyMsisdn, medium: "msisdn"}], @@ -171,8 +185,9 @@ export default class PhoneNumbers extends React.Component { verifying: false, verifyMsisdn: "", verifyError: null, + newPhoneNumber: "", + newPhoneNumberCode: "", }); - this.refs.newPhoneNumber.value = ""; }).catch((err) => { this.setState({continueDisabled: false}); if (err.errcode !== 'M_THREEPID_AUTH_FAILED') { @@ -213,8 +228,14 @@ export default class PhoneNumbers extends React.Component { {this.state.verifyError}
- + {_t("Continue")} @@ -238,9 +259,15 @@ export default class PhoneNumbers extends React.Component {
- +
{addVerifySection} From 69a066657bbd060ca93868ecfd730625c7b0d331 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 12 Mar 2019 16:51:19 +0000 Subject: [PATCH 10/11] Remove bits of Field that supported uncontrolled Field is no longer used as an uncontrolled component, so we can remove some supporting code that we no longer need. --- src/components/views/elements/Field.js | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index cd06610c62c..daf6ec0ce19 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -53,22 +53,9 @@ export default class Field extends React.PureComponent { }; } - /* TODO: Remove me */ - get value() { - if (!this.refs.fieldInput) return null; - return this.refs.fieldInput.value; - } - - set value(newValue) { - if (!this.refs.fieldInput) { - throw new Error("No field input reference"); - } - this.refs.fieldInput.value = newValue; - } - onChange = (ev) => { if (this.props.onValidate) { - const result = this.props.onValidate(this.value); + const result = this.props.onValidate(ev.target.value); this.setState({ valid: result.valid, feedback: result.feedback, @@ -92,12 +79,6 @@ export default class Field extends React.PureComponent { inputProps.onChange = this.onChange; - /* TODO: Remove me */ - // make sure we use the current `value` for the field and not the original one - if (inputProps.value === undefined) { - inputProps.value = this.value || ""; - } - const fieldInput = React.createElement(inputElement, inputProps, children); let prefixContainer = null; From e90d659e195e1dfdf94c1840572bff634d224825 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 12 Mar 2019 16:53:30 +0000 Subject: [PATCH 11/11] Remove validation demo code This is example code from @ara4n's work in https://github.com/matrix-org/matrix-react-sdk/pull/2550. We're not ready to actually apply validation yet, so removing this for now. --- src/components/views/auth/ServerConfig.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/components/views/auth/ServerConfig.js b/src/components/views/auth/ServerConfig.js index ed6b4bdf7dc..cb0e0dc38ec 100644 --- a/src/components/views/auth/ServerConfig.js +++ b/src/components/views/auth/ServerConfig.js @@ -90,15 +90,6 @@ export default class ServerConfig extends React.PureComponent { this.setState({ hsUrl }); } - onHomeserverValidate = (value) => { - try { - new URL(value); - return { valid: true, feedback:
Valid URL!
}; - } catch (_) { - return { valid: false, feedback:
Invalid URL!
}; - } - } - onIdentityServerBlur = (ev) => { this._isTimeoutId = this._waitThenInvoke(this._isTimeoutId, () => { this.props.onServerConfigChange({ @@ -143,7 +134,6 @@ export default class ServerConfig extends React.PureComponent { value={this.state.hsUrl} onBlur={this.onHomeserverBlur} onChange={this.onHomeserverChange} - onValidate={this.onHomeserverValidate} />