From 10caf59b5a255f58ceb6b95eb72051921fdb9751 Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Mon, 26 Jul 2021 13:54:17 +0300
Subject: [PATCH] Refactor FormField component (#17089)
* Improve prop-types, move to the top of the file
* Make FormField a functional component
* Simplify default props assignment
* Remove redundant comparisons
* Add InputContainer component
* Extract ALL_DOT_CHARS regexp for form field id
* Simplify Label
* Extract rootClassNames
* Fix formFieldId
* Fix FormField prop type
---
.../metabase/components/form/FormField.jsx | 176 +++++++++---------
.../components/form/FormField.styled.js | 18 +-
2 files changed, 105 insertions(+), 89 deletions(-)
diff --git a/frontend/src/metabase/components/form/FormField.jsx b/frontend/src/metabase/components/form/FormField.jsx
index 945ef58f70d..360a0c1db9f 100644
--- a/frontend/src/metabase/components/form/FormField.jsx
+++ b/frontend/src/metabase/components/form/FormField.jsx
@@ -1,100 +1,102 @@
-/* eslint-disable react/prop-types */
-import React, { Component } from "react";
+import React from "react";
import PropTypes from "prop-types";
import cx from "classnames";
import Tooltip from "metabase/components/Tooltip";
-import { FieldRow, Label, InfoIcon } from "./FormField.styled";
+import { FieldRow, Label, InfoIcon, InputContainer } from "./FormField.styled";
-export default class FormField extends Component {
- static propTypes = {
- field: PropTypes.object,
- formField: PropTypes.object,
+const formFieldCommon = {
+ title: PropTypes.string,
+ description: PropTypes.string,
+ info: PropTypes.string,
+ hidden: PropTypes.bool,
+ horizontal: PropTypes.bool,
+};
- // redux-form compatible:
- name: PropTypes.string,
- error: PropTypes.any,
- visited: PropTypes.bool,
- active: PropTypes.bool,
+const propTypes = {
+ ...formFieldCommon,
- hidden: PropTypes.bool,
- title: PropTypes.string,
- description: PropTypes.string,
- info: PropTypes.string,
+ field: PropTypes.object,
+ formField: PropTypes.shape({
+ ...formFieldCommon,
+ type: PropTypes.oneOfType([PropTypes.string, PropTypes.func]),
+ }),
- children: PropTypes.oneOfType([
- PropTypes.arrayOf(PropTypes.node),
- PropTypes.node,
- ]),
+ // redux-form compatible:
+ name: PropTypes.string,
+ error: PropTypes.any,
+ visited: PropTypes.bool,
+ active: PropTypes.bool,
+
+ children: PropTypes.oneOfType([
+ PropTypes.arrayOf(PropTypes.node),
+ PropTypes.node,
+ ]),
+ className: PropTypes.string,
+};
+
+const ALL_DOT_CHARS = /\./g;
+
+function FormField(props) {
+ const {
+ className,
+ formField,
+ title = formField && formField.title,
+ description = formField && formField.description,
+ info = formField && formField.info,
+ hidden = formField && (formField.hidden || formField.type === "hidden"),
+ horizontal = formField &&
+ (formField.horizontal || formField.type === "boolean"),
+ children,
+ } = props;
+
+ if (hidden) {
+ return null;
+ }
+
+ let { name, error, visited, active } = {
+ ...(props.field || {}),
+ ...props,
};
- render() {
- const {
- className,
- formField,
- title = formField && formField.title,
- description = formField && formField.description,
- info = formField && formField.info,
- hidden = formField &&
- (formField.hidden != null
- ? formField.hidden
- : formField.type === "hidden"),
- horizontal = formField &&
- (formField.horizontal != null
- ? formField.horizontal
- : formField.type === "boolean"),
- children,
- } = this.props;
-
- if (hidden) {
- return null;
- }
-
- let { name, error, visited, active } = {
- ...(this.props.field || {}),
- ...this.props,
- };
-
- if (visited === false || active === true) {
- // if the field hasn't been visited or is currently active then don't show the error
- error = null;
- }
-
- return (
- <div
- className={cx("Form-field", className, {
- "Form--fieldError": !!error,
- flex: horizontal,
- })}
- id={`formField-${name.replace(/\./g, "-")}`}
- >
- {(title || description) && (
- <div>
- <FieldRow>
- {title && (
- <Label
- className={cx("Form-label", { "mr-auto": horizontal })}
- htmlFor={name}
- id={`${name}-label`}
- >
- {title}
- {error && <span className="text-error">: {error}</span>}
- </Label>
- )}
- {info && (
- <Tooltip tooltip={info}>
- <InfoIcon />
- </Tooltip>
- )}
- </FieldRow>
- {description && <div className="mb1">{description}</div>}
- </div>
- )}
- <div className={cx("flex-no-shrink", { "ml-auto": horizontal })}>
- {children}
- </div>
- </div>
- );
+ const formFieldId = `formField-${name.replace(ALL_DOT_CHARS, "-")}`;
+
+ if (!visited || active) {
+ // if the field hasn't been visited or is currently active then don't show the error
+ error = null;
}
+
+ const rootClassNames = cx("Form-field", className, {
+ "Form--fieldError": !!error,
+ flex: horizontal,
+ });
+
+ return (
+ <div id={formFieldId} className={rootClassNames}>
+ {(title || description) && (
+ <div>
+ <FieldRow>
+ {title && (
+ <Label id={`${name}-label`} htmlFor={name} horizontal>
+ {title}
+ {error && <span className="text-error">: {error}</span>}
+ </Label>
+ )}
+ {info && (
+ <Tooltip tooltip={info}>
+ <InfoIcon />
+ </Tooltip>
+ )}
+ </FieldRow>
+ {description && <div className="mb1">{description}</div>}
+ </div>
+ )}
+ <InputContainer horizontal={horizontal}>{children}</InputContainer>
+ </div>
+ );
}
+
+FormField.propTypes = propTypes;
+
+export default FormField;
diff --git a/frontend/src/metabase/components/form/FormField.styled.js b/frontend/src/metabase/components/form/FormField.styled.js
index 656369536b3..3d46778225d 100644
--- a/frontend/src/metabase/components/form/FormField.styled.js
+++ b/frontend/src/metabase/components/form/FormField.styled.js
@@ -1,4 +1,4 @@
-import styled from "styled-components";
+import styled, { css } from "styled-components";
import Icon from "metabase/components/Icon";
import { color } from "metabase/lib/colors";
@@ -9,8 +9,13 @@ export const FieldRow = styled.div`
margin-bottom: 0.5em;
`;
-export const Label = styled.label`
+export const Label = styled.label.attrs({ className: "Form-label" })`
margin-bottom: 0;
+ ${props =>
+ props.horizontal &&
+ css`
+ margin-right: auto;
+ `}
`;
export const InfoIcon = styled(Icon).attrs({ name: "info", size: 12 })`
@@ -21,3 +26,12 @@ export const InfoIcon = styled(Icon).attrs({ name: "info", size: 12 })`
color: ${color("brand")};
}
`;
+
+export const InputContainer = styled.div`
+ flex-shrink: 0;
+ ${props =>
+ props.horizontal &&
+ css`
+ margin-left: auto;
+ `}
+`;
--
GitLab