From f9b45677d60224e5f3876c447e1f2ab006b3f100 Mon Sep 17 00:00:00 2001
From: David Baker <dave@matrix.org>
Date: Fri, 16 Jul 2021 22:27:31 +0100
Subject: [PATCH 1/2] Fix bug where 'other homeserver' would unfocus
It turns out the answer to this was not all that complex: we had
two nested <label>s: one for the radio button and one for the Field,
which presumably causes both of them to generate click events and
focus something and what ends up focused is some kind of race condition
depending on, apparently, how fast you click.
Notes: Fix bug where the 'other homeserver' field in the server selection dialog would become briefly focus and then unfocus when clicked.
---
.../views/elements/_StyledRadioButton.scss | 8 ++++-
.../views/dialogs/ServerPickerDialog.tsx | 3 +-
.../views/elements/StyledRadioButton.tsx | 30 +++++++++++++++----
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/res/css/views/elements/_StyledRadioButton.scss b/res/css/views/elements/_StyledRadioButton.scss
index 62fb5c5512..1ae787dfc2 100644
--- a/res/css/views/elements/_StyledRadioButton.scss
+++ b/res/css/views/elements/_StyledRadioButton.scss
@@ -46,7 +46,7 @@ limitations under the License.
width: $font-16px;
}
- > input[type=radio] {
+ input[type=radio] {
// Remove the OS's representation
margin: 0;
padding: 0;
@@ -112,6 +112,12 @@ limitations under the License.
}
}
}
+
+ .mx_RadioButton_innerLabel {
+ display: flex;
+ position: relative;
+ top: 4px;
+ }
}
.mx_RadioButton_outlined {
diff --git a/src/components/views/dialogs/ServerPickerDialog.tsx b/src/components/views/dialogs/ServerPickerDialog.tsx
index 8dafd8a2bc..d480ca2043 100644
--- a/src/components/views/dialogs/ServerPickerDialog.tsx
+++ b/src/components/views/dialogs/ServerPickerDialog.tsx
@@ -205,13 +205,14 @@ export default class ServerPickerDialog extends React.PureComponent<IProps, ISta
className="mx_ServerPickerDialog_otherHomeserverRadio"
checked={!this.state.defaultChosen}
onChange={this.onOtherChosen}
+ childrenInLabel={false}
>
<Field
type="text"
className="mx_ServerPickerDialog_otherHomeserver"
label={_t("Other homeserver")}
onChange={this.onHomeserverChange}
- onClick={this.onOtherChosen}
+ onFocus={this.onOtherChosen}
ref={this.fieldRef}
onValidate={this.onHomeserverValidate}
value={this.state.otherHomeserver}
diff --git a/src/components/views/elements/StyledRadioButton.tsx b/src/components/views/elements/StyledRadioButton.tsx
index 7ec472b639..d2edffd5b6 100644
--- a/src/components/views/elements/StyledRadioButton.tsx
+++ b/src/components/views/elements/StyledRadioButton.tsx
@@ -20,6 +20,10 @@ import { replaceableComponent } from "../../../utils/replaceableComponent";
interface IProps extends React.InputHTMLAttributes<HTMLInputElement> {
outlined?: boolean;
+ // If true (default), the children will be contained within a <label> element
+ // If false, they'll be in a div. Putting interactive components like Fields in labels can
+ // cause strange bugs like https://github.com/vector-im/element-web/issues/18031
+ childrenInLabel?: boolean;
}
interface IState {
@@ -29,10 +33,11 @@ interface IState {
export default class StyledRadioButton extends React.PureComponent<IProps, IState> {
public static readonly defaultProps = {
className: '',
+ childrenInLabel: true,
};
public render() {
- const { children, className, disabled, outlined, ...otherProps } = this.props;
+ const { children, className, disabled, outlined, childrenInLabel, ...otherProps } = this.props;
const _className = classnames(
'mx_RadioButton',
className,
@@ -42,12 +47,27 @@ export default class StyledRadioButton extends React.PureComponent<IProps, IStat
"mx_RadioButton_checked": this.props.checked,
"mx_RadioButton_outlined": outlined,
});
- return <label className={_className}>
+
+ const radioButton = <React.Fragment>
<input type='radio' disabled={disabled} {...otherProps} />
{/* Used to render the radio button circle */}
<div><div /></div>
- <div className="mx_RadioButton_content">{children}</div>
- <div className="mx_RadioButton_spacer" />
- </label>;
+ </React.Fragment>;
+
+ if (childrenInLabel) {
+ return <label className={_className}>
+ {radioButton}
+ <div className="mx_RadioButton_content">{children}</div>
+ <div className="mx_RadioButton_spacer" />
+ </label>;
+ } else {
+ return <div className={_className}>
+ <label className="mx_RadioButton_innerLabel">
+ {radioButton}
+ </label>
+ <div className="mx_RadioButton_content">{children}</div>
+ <div className="mx_RadioButton_spacer" />
+ </div>;
+ }
}
}
From 45fb0e927f9c2bd330ca34eda56918d99987d5dd Mon Sep 17 00:00:00 2001
From: David Baker <dave@matrix.org>
Date: Fri, 16 Jul 2021 22:35:53 +0100
Subject: [PATCH 2/2] Mention nested labels in comments
---
src/components/views/elements/StyledRadioButton.tsx | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/components/views/elements/StyledRadioButton.tsx b/src/components/views/elements/StyledRadioButton.tsx
index d2edffd5b6..38d7358524 100644
--- a/src/components/views/elements/StyledRadioButton.tsx
+++ b/src/components/views/elements/StyledRadioButton.tsx
@@ -21,8 +21,8 @@ import { replaceableComponent } from "../../../utils/replaceableComponent";
interface IProps extends React.InputHTMLAttributes<HTMLInputElement> {
outlined?: boolean;
// If true (default), the children will be contained within a <label> element
- // If false, they'll be in a div. Putting interactive components like Fields in labels can
- // cause strange bugs like https://github.com/vector-im/element-web/issues/18031
+ // If false, they'll be in a div. Putting interactive components that have labels
+ // themselves in labels can cause strange bugs like https://github.com/vector-im/element-web/issues/18031
childrenInLabel?: boolean;
}