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.

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

Incorrect password error in a poor location in the Add Phone Number dialog #19345

Closed
kittykat opened this issue Oct 11, 2021 · 4 comments · Fixed by matrix-org/matrix-react-sdk#6961
Labels
A-3PIDs good first issue Good for newcomers Hacktoberfest Issues which are suitable for Hacktoberfest PRs: https://hacktoberfest.digitalocean.com/ Help Wanted Extra attention is needed O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Low/no impact on users T-Enhancement

Comments

@kittykat
Copy link
Contributor

Steps to reproduce

  1. Settings -> add phone number
  2. Enter verification code -> Continue
  3. Type in wrong password

Outcome

What did you expect?

The error message to be in a better location (maybe under or next to password field?). Dialog does not resize if wrong password is submitted.

What happened instead?

Error message is poorly located and resized dialog.

Screenshot from 2021-10-11 09-32-21

Operating system

No response

Browser information

No response

URL for webapp

develop.element.io

Homeserver

No response

Will you send logs?

No

@kittykat kittykat added T-Defect S-Tolerable Low/no impact on users X-Needs-Design A-3PIDs O-Occasional Affects or can be seen by some users regularly or most users rarely T-Enhancement and removed T-Defect labels Oct 11, 2021
@dbkr
Copy link
Member

dbkr commented Oct 11, 2021

I'm going to go out on a limb & say the field component is actually standard enough that we shouldn't need additional design work for this - the password field should just go to its standard 'invalid' state (the PR should still go via design for approval of course).

@dbkr dbkr added good first issue Good for newcomers Help Wanted Extra attention is needed and removed X-Needs-Design labels Oct 11, 2021
@t3chguy
Copy link
Member

t3chguy commented Oct 11, 2021

+1 though mixing parent state and field validation can be a PITA currently

@t3chguy
Copy link
Member

t3chguy commented Oct 11, 2021

I may steal this one just because im in that exact area now for #18883

@t3chguy
Copy link
Member

t3chguy commented Oct 11, 2021

image

Not going to include it as part of the same work due to it needing design review

Index: res/css/views/elements/_Field.scss
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/res/css/views/elements/_Field.scss b/res/css/views/elements/_Field.scss
--- a/res/css/views/elements/_Field.scss	(revision 79a46782d6cc72fbd9018de6ea8c54c97afba55c)
+++ b/res/css/views/elements/_Field.scss	(date 1633959573079)
@@ -175,7 +175,8 @@
 .mx_Field_tooltip {
     margin-top: -12px;
     margin-left: 4px;
-    width: 200px;
+    max-width: 200px;
+    width: auto;
 }
 
 .mx_Field_tooltip.mx_Field_valid {
Index: src/components/views/auth/InteractiveAuthEntryComponents.tsx
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/components/views/auth/InteractiveAuthEntryComponents.tsx b/src/components/views/auth/InteractiveAuthEntryComponents.tsx
--- a/src/components/views/auth/InteractiveAuthEntryComponents.tsx	(revision 0e7b47c5c1984760808a5273719802dd367401e1)
+++ b/src/components/views/auth/InteractiveAuthEntryComponents.tsx	(date 1633959385355)
@@ -136,10 +136,6 @@
     };
 
     render() {
-        const passwordBoxClass = classNames({
-            "error": this.props.errorText,
-        });
-
         let submitButtonOrSpinner;
         if (this.props.busy) {
             submitButtonOrSpinner = <Spinner />;
@@ -153,27 +149,20 @@
             );
         }
 
-        let errorSection;
-        if (this.props.errorText) {
-            errorSection = (
-                <div className="error" role="alert">
-                    { this.props.errorText }
-                </div>
-            );
-        }
-
         return (
             <div>
                 <p>{ _t("Confirm your identity by entering your account password below.") }</p>
                 <form onSubmit={this.onSubmit} className="mx_InteractiveAuthEntryComponents_passwordSection">
                     <Field
-                        className={passwordBoxClass}
                         type="password"
                         name="passwordField"
                         label={_t('Password')}
                         autoFocus={true}
                         value={this.state.password}
                         onChange={this.onPasswordFieldChange}
+                        forceTooltipVisible={!!this.props.errorText}
+                        forceValidity={this.props.errorText ? false : undefined}
+                        tooltipContent={this.props.errorText}
                     />
                     <div className="mx_button_row">
                         { submitButtonOrSpinner }
Index: src/components/views/elements/Field.tsx
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx
--- a/src/components/views/elements/Field.tsx	(revision 79a46782d6cc72fbd9018de6ea8c54c97afba55c)
+++ b/src/components/views/elements/Field.tsx	(date 1633959518885)
@@ -220,7 +220,7 @@
 
     public render() {
         /* eslint @typescript-eslint/no-unused-vars: ["error", { "ignoreRestSiblings": true }] */
-        const { element, prefixComponent, postfixComponent, className, onValidate, children,
+        const { element, prefixComponent, postfixComponent, className, onValidate, children, forceTooltipVisible,
             tooltipContent, forceValidity, tooltipClassName, list, validateOnBlur, validateOnChange, validateOnFocus,
             ...inputProps } = this.props;
 
@@ -266,7 +266,7 @@
         if (tooltipContent || this.state.feedback) {
             fieldTooltip = <Tooltip
                 tooltipClassName={classNames("mx_Field_tooltip", tooltipClassName)}
-                visible={(this.state.focused && this.props.forceTooltipVisible) || this.state.feedbackVisible}
+                visible={forceTooltipVisible || this.state.feedbackVisible}
                 label={tooltipContent || this.state.feedback}
                 alignment={Tooltip.Alignment.Right}
             />;

@kittykat kittykat added the Hacktoberfest Issues which are suitable for Hacktoberfest PRs: https://hacktoberfest.digitalocean.com/ label Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-3PIDs good first issue Good for newcomers Hacktoberfest Issues which are suitable for Hacktoberfest PRs: https://hacktoberfest.digitalocean.com/ Help Wanted Extra attention is needed O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Low/no impact on users T-Enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants