From a0af4869245f72725238518718840443dc6b3c94 Mon Sep 17 00:00:00 2001 From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com> Date: Fri, 9 Jul 2021 13:10:14 +0200 Subject: [PATCH 001/664] Remove `recompose` (#16829) * Replace `pure` with `React.Memo` * Fix eslint error * Use `shallowEqual` directly as a utility function * Remove `recompose` library --- .../widgets/CustomGeoJSONWidget.jsx | 6 +- frontend/src/metabase/components/List.jsx | 3 +- frontend/src/metabase/components/ListItem.jsx | 3 +- .../src/metabase/components/QueryButton.jsx | 3 +- .../src/metabase/components/SidebarItem.jsx | 4 +- .../components/TitleAndDescription.jsx | 3 +- .../metabase/components/TooltipPopover.jsx | 3 +- .../metabase/reference/components/Detail.jsx | 3 +- .../reference/components/EditHeader.jsx | 3 +- .../components/EditableReferenceHeader.jsx | 3 +- .../metabase/reference/components/Field.jsx | 3 +- .../reference/components/FieldToGroupBy.jsx | 3 +- .../reference/components/FieldTypeDetail.jsx | 3 +- .../reference/components/ReferenceHeader.jsx | 3 +- .../reference/components/UsefulQuestions.jsx | 3 +- .../reference/databases/DatabaseSidebar.jsx | 3 +- .../reference/databases/FieldSidebar.jsx | 3 +- .../reference/databases/TableSidebar.jsx | 3 +- .../metabase/reference/guide/BaseSidebar.jsx | 3 +- .../reference/metrics/MetricSidebar.jsx | 3 +- .../segments/SegmentFieldSidebar.jsx | 3 +- .../reference/segments/SegmentSidebar.jsx | 3 +- frontend/src/metabase/selectors/metadata.js | 2 +- .../src/metabase/selectors/shallowEqual.js | 76 +++++++++++++++++++ package.json | 1 - yarn.lock | 30 +------- 26 files changed, 104 insertions(+), 75 deletions(-) create mode 100644 frontend/src/metabase/selectors/shallowEqual.js diff --git a/frontend/src/metabase/admin/settings/components/widgets/CustomGeoJSONWidget.jsx b/frontend/src/metabase/admin/settings/components/widgets/CustomGeoJSONWidget.jsx index 270158d5c73d..148dbad1b78c 100644 --- a/frontend/src/metabase/admin/settings/components/widgets/CustomGeoJSONWidget.jsx +++ b/frontend/src/metabase/admin/settings/components/widgets/CustomGeoJSONWidget.jsx @@ -18,8 +18,6 @@ import cx from "classnames"; import LeafletChoropleth from "metabase/visualizations/components/LeafletChoropleth"; -import pure from "recompose/pure"; - export default class CustomGeoJSONWidget extends Component { constructor(props, context) { super(props, context); @@ -411,6 +409,8 @@ const EditMap = ({ ); -const ChoroplethPreview = pure(({ geoJson }) => ( +const ChoroplethPreview = React.memo(({ geoJson }) => ( )); + +ChoroplethPreview.displayName = "ChoroplethPreview"; diff --git a/frontend/src/metabase/components/List.jsx b/frontend/src/metabase/components/List.jsx index d94461d10caa..1ece4cc4be1b 100644 --- a/frontend/src/metabase/components/List.jsx +++ b/frontend/src/metabase/components/List.jsx @@ -3,7 +3,6 @@ import React from "react"; import PropTypes from "prop-types"; import S from "./List.css"; -import pure from "recompose/pure"; const List = ({ children }) => ; @@ -11,4 +10,4 @@ List.propTypes = { children: PropTypes.any.isRequired, }; -export default pure(List); +export default React.memo(List); diff --git a/frontend/src/metabase/components/ListItem.jsx b/frontend/src/metabase/components/ListItem.jsx index 656e49daf1c2..62cea41c26a6 100644 --- a/frontend/src/metabase/components/ListItem.jsx +++ b/frontend/src/metabase/components/ListItem.jsx @@ -7,7 +7,6 @@ import Icon from "./Icon"; import Ellipsified from "./Ellipsified"; import cx from "classnames"; -import pure from "recompose/pure"; import Card from "metabase/components/Card"; //TODO: extend this to support functionality required for questions @@ -50,4 +49,4 @@ ListItem.propTypes = { field: PropTypes.object, }; -export default pure(ListItem); +export default React.memo(ListItem); diff --git a/frontend/src/metabase/components/QueryButton.jsx b/frontend/src/metabase/components/QueryButton.jsx index dfcb11ba72ba..7f519458885e 100644 --- a/frontend/src/metabase/components/QueryButton.jsx +++ b/frontend/src/metabase/components/QueryButton.jsx @@ -1,7 +1,6 @@ import React from "react"; import PropTypes from "prop-types"; import { Link } from "react-router"; -import pure from "recompose/pure"; import cx from "classnames"; import S from "./QueryButton.css"; @@ -29,4 +28,4 @@ QueryButton.propTypes = { link: PropTypes.string, }; -export default pure(QueryButton); +export default React.memo(QueryButton); diff --git a/frontend/src/metabase/components/SidebarItem.jsx b/frontend/src/metabase/components/SidebarItem.jsx index fb3684839388..d3ff05d2284c 100644 --- a/frontend/src/metabase/components/SidebarItem.jsx +++ b/frontend/src/metabase/components/SidebarItem.jsx @@ -6,8 +6,6 @@ import S from "./Sidebar.css"; import LabelIcon from "./LabelIcon"; -import pure from "recompose/pure"; - const SidebarItem = ({ name, sidebar, icon, href }) => (
  • @@ -24,4 +22,4 @@ SidebarItem.propTypes = { href: PropTypes.string.isRequired, }; -export default pure(SidebarItem); +export default React.memo(SidebarItem); diff --git a/frontend/src/metabase/components/TitleAndDescription.jsx b/frontend/src/metabase/components/TitleAndDescription.jsx index e7378a379e8d..1a628816cda4 100644 --- a/frontend/src/metabase/components/TitleAndDescription.jsx +++ b/frontend/src/metabase/components/TitleAndDescription.jsx @@ -1,6 +1,5 @@ import React from "react"; import cx from "classnames"; -import pure from "recompose/pure"; import Icon from "metabase/components/Icon"; import Tooltip from "metabase/components/Tooltip"; @@ -21,4 +20,4 @@ const TitleAndDescription = ({ title, description, className }: Attributes) => ( ); -export default pure(TitleAndDescription); +export default React.memo(TitleAndDescription); diff --git a/frontend/src/metabase/components/TooltipPopover.jsx b/frontend/src/metabase/components/TooltipPopover.jsx index 5a92a330d288..d706955c17f1 100644 --- a/frontend/src/metabase/components/TooltipPopover.jsx +++ b/frontend/src/metabase/components/TooltipPopover.jsx @@ -1,6 +1,5 @@ /* eslint-disable react/prop-types */ import React from "react"; -import pure from "recompose/pure"; import cx from "classnames"; import Popover from "./Popover"; @@ -38,4 +37,4 @@ TooltipPopover.defaultProps = { constrainedWidth: true, }; -export default pure(TooltipPopover); +export default React.memo(TooltipPopover); diff --git a/frontend/src/metabase/reference/components/Detail.jsx b/frontend/src/metabase/reference/components/Detail.jsx index 8735684875f1..bda51a8ccaa3 100644 --- a/frontend/src/metabase/reference/components/Detail.jsx +++ b/frontend/src/metabase/reference/components/Detail.jsx @@ -5,7 +5,6 @@ import { Link } from "react-router"; import S from "./Detail.css"; import { t } from "ttag"; import cx from "classnames"; -import pure from "recompose/pure"; const Detail = ({ name, @@ -64,4 +63,4 @@ Detail.propTypes = { field: PropTypes.object, }; -export default pure(Detail); +export default React.memo(Detail); diff --git a/frontend/src/metabase/reference/components/EditHeader.jsx b/frontend/src/metabase/reference/components/EditHeader.jsx index 04c2c6239dbd..4aea2c1f6c0d 100644 --- a/frontend/src/metabase/reference/components/EditHeader.jsx +++ b/frontend/src/metabase/reference/components/EditHeader.jsx @@ -1,7 +1,6 @@ import React from "react"; import PropTypes from "prop-types"; import cx from "classnames"; -import pure from "recompose/pure"; import { t } from "ttag"; import S from "./EditHeader.css"; @@ -81,4 +80,4 @@ EditHeader.propTypes = { revisionMessageFormField: PropTypes.object, }; -export default pure(EditHeader); +export default React.memo(EditHeader); diff --git a/frontend/src/metabase/reference/components/EditableReferenceHeader.jsx b/frontend/src/metabase/reference/components/EditableReferenceHeader.jsx index 2872191056af..a3377baf38af 100644 --- a/frontend/src/metabase/reference/components/EditableReferenceHeader.jsx +++ b/frontend/src/metabase/reference/components/EditableReferenceHeader.jsx @@ -2,7 +2,6 @@ import React from "react"; import PropTypes from "prop-types"; import { Link } from "react-router"; import cx from "classnames"; -import pure from "recompose/pure"; import { t } from "ttag"; import S from "./ReferenceHeader.css"; import L from "metabase/components/List.css"; @@ -110,4 +109,4 @@ EditableReferenceHeader.propTypes = { nameFormField: PropTypes.object, }; -export default pure(EditableReferenceHeader); +export default React.memo(EditableReferenceHeader); diff --git a/frontend/src/metabase/reference/components/Field.jsx b/frontend/src/metabase/reference/components/Field.jsx index ce745e6ccc85..1914a17e689c 100644 --- a/frontend/src/metabase/reference/components/Field.jsx +++ b/frontend/src/metabase/reference/components/Field.jsx @@ -15,7 +15,6 @@ import Select from "metabase/components/Select"; import Icon from "metabase/components/Icon"; import cx from "classnames"; -import pure from "recompose/pure"; const Field = ({ field, foreignKeys, url, icon, isEditing, formField }) => (
    @@ -127,4 +126,4 @@ Field.propTypes = { formField: PropTypes.object, }; -export default pure(Field); +export default React.memo(Field); diff --git a/frontend/src/metabase/reference/components/FieldToGroupBy.jsx b/frontend/src/metabase/reference/components/FieldToGroupBy.jsx index 4d67308e6612..8423c7b17429 100644 --- a/frontend/src/metabase/reference/components/FieldToGroupBy.jsx +++ b/frontend/src/metabase/reference/components/FieldToGroupBy.jsx @@ -1,7 +1,6 @@ /* eslint-disable react/prop-types */ import React from "react"; import PropTypes from "prop-types"; -import pure from "recompose/pure"; import { t } from "ttag"; import cx from "classnames"; import S from "./FieldToGroupBy.css"; @@ -42,4 +41,4 @@ FieldToGroupBy.propTypes = { secondaryOnClick: PropTypes.func, }; -export default pure(FieldToGroupBy); +export default React.memo(FieldToGroupBy); diff --git a/frontend/src/metabase/reference/components/FieldTypeDetail.jsx b/frontend/src/metabase/reference/components/FieldTypeDetail.jsx index 6dfca7fe97b4..4b044a26eb59 100644 --- a/frontend/src/metabase/reference/components/FieldTypeDetail.jsx +++ b/frontend/src/metabase/reference/components/FieldTypeDetail.jsx @@ -2,7 +2,6 @@ import React from "react"; import PropTypes from "prop-types"; import cx from "classnames"; import { getIn } from "icepick"; -import pure from "recompose/pure"; import { t } from "ttag"; import * as MetabaseCore from "metabase/lib/core"; import { isNumericBaseType } from "metabase/lib/schema_metadata"; @@ -88,4 +87,4 @@ FieldTypeDetail.propTypes = { isEditing: PropTypes.bool.isRequired, }; -export default pure(FieldTypeDetail); +export default React.memo(FieldTypeDetail); diff --git a/frontend/src/metabase/reference/components/ReferenceHeader.jsx b/frontend/src/metabase/reference/components/ReferenceHeader.jsx index 78a4f4c088ed..4db9ea67af22 100644 --- a/frontend/src/metabase/reference/components/ReferenceHeader.jsx +++ b/frontend/src/metabase/reference/components/ReferenceHeader.jsx @@ -2,7 +2,6 @@ import React from "react"; import PropTypes from "prop-types"; import { Link } from "react-router"; import cx from "classnames"; -import pure from "recompose/pure"; import S from "./ReferenceHeader.css"; import L from "metabase/components/List.css"; @@ -61,4 +60,4 @@ ReferenceHeader.propTypes = { headerLink: PropTypes.string, }; -export default pure(ReferenceHeader); +export default React.memo(ReferenceHeader); diff --git a/frontend/src/metabase/reference/components/UsefulQuestions.jsx b/frontend/src/metabase/reference/components/UsefulQuestions.jsx index e2d31bc7fa5d..9fa1144804fd 100644 --- a/frontend/src/metabase/reference/components/UsefulQuestions.jsx +++ b/frontend/src/metabase/reference/components/UsefulQuestions.jsx @@ -1,6 +1,5 @@ import React from "react"; import PropTypes from "prop-types"; -import pure from "recompose/pure"; import { t } from "ttag"; import S from "./UsefulQuestions.css"; import D from "metabase/reference/components/Detail.css"; @@ -25,4 +24,4 @@ UsefulQuestions.propTypes = { questions: PropTypes.array.isRequired, }; -export default pure(UsefulQuestions); +export default React.memo(UsefulQuestions); diff --git a/frontend/src/metabase/reference/databases/DatabaseSidebar.jsx b/frontend/src/metabase/reference/databases/DatabaseSidebar.jsx index 679b1b9f6bd3..7d1a0b8b2324 100644 --- a/frontend/src/metabase/reference/databases/DatabaseSidebar.jsx +++ b/frontend/src/metabase/reference/databases/DatabaseSidebar.jsx @@ -7,7 +7,6 @@ import Breadcrumbs from "metabase/components/Breadcrumbs"; import SidebarItem from "metabase/components/SidebarItem"; import cx from "classnames"; -import pure from "recompose/pure"; const DatabaseSidebar = ({ database, style, className }) => (
    @@ -43,4 +42,4 @@ DatabaseSidebar.propTypes = { style: PropTypes.object, }; -export default pure(DatabaseSidebar); +export default React.memo(DatabaseSidebar); diff --git a/frontend/src/metabase/reference/databases/FieldSidebar.jsx b/frontend/src/metabase/reference/databases/FieldSidebar.jsx index fdc8bb550724..59bbca8e9167 100644 --- a/frontend/src/metabase/reference/databases/FieldSidebar.jsx +++ b/frontend/src/metabase/reference/databases/FieldSidebar.jsx @@ -3,7 +3,6 @@ import React from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; import cx from "classnames"; -import pure from "recompose/pure"; import MetabaseSettings from "metabase/lib/settings"; @@ -59,4 +58,4 @@ FieldSidebar.propTypes = { style: PropTypes.object, }; -export default pure(FieldSidebar); +export default React.memo(FieldSidebar); diff --git a/frontend/src/metabase/reference/databases/TableSidebar.jsx b/frontend/src/metabase/reference/databases/TableSidebar.jsx index b7b581a2be7a..b20bf1538fab 100644 --- a/frontend/src/metabase/reference/databases/TableSidebar.jsx +++ b/frontend/src/metabase/reference/databases/TableSidebar.jsx @@ -3,7 +3,6 @@ import React from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; import cx from "classnames"; -import pure from "recompose/pure"; import MetabaseSettings from "metabase/lib/settings"; @@ -64,4 +63,4 @@ TableSidebar.propTypes = { style: PropTypes.object, }; -export default pure(TableSidebar); +export default React.memo(TableSidebar); diff --git a/frontend/src/metabase/reference/guide/BaseSidebar.jsx b/frontend/src/metabase/reference/guide/BaseSidebar.jsx index 3079c7758a9b..9c7e42fdc385 100644 --- a/frontend/src/metabase/reference/guide/BaseSidebar.jsx +++ b/frontend/src/metabase/reference/guide/BaseSidebar.jsx @@ -7,7 +7,6 @@ import Breadcrumbs from "metabase/components/Breadcrumbs"; import SidebarItem from "metabase/components/SidebarItem"; import cx from "classnames"; -import pure from "recompose/pure"; const BaseSidebar = ({ style, className }) => (
    @@ -47,4 +46,4 @@ BaseSidebar.propTypes = { style: PropTypes.object, }; -export default pure(BaseSidebar); +export default React.memo(BaseSidebar); diff --git a/frontend/src/metabase/reference/metrics/MetricSidebar.jsx b/frontend/src/metabase/reference/metrics/MetricSidebar.jsx index 8c090fd5b087..dc44ffdc3eb3 100644 --- a/frontend/src/metabase/reference/metrics/MetricSidebar.jsx +++ b/frontend/src/metabase/reference/metrics/MetricSidebar.jsx @@ -3,7 +3,6 @@ import React from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; import cx from "classnames"; -import pure from "recompose/pure"; import MetabaseSettings from "metabase/lib/settings"; @@ -64,4 +63,4 @@ MetricSidebar.propTypes = { style: PropTypes.object, }; -export default pure(MetricSidebar); +export default React.memo(MetricSidebar); diff --git a/frontend/src/metabase/reference/segments/SegmentFieldSidebar.jsx b/frontend/src/metabase/reference/segments/SegmentFieldSidebar.jsx index 208e92b42b27..997d9d85ac23 100644 --- a/frontend/src/metabase/reference/segments/SegmentFieldSidebar.jsx +++ b/frontend/src/metabase/reference/segments/SegmentFieldSidebar.jsx @@ -7,7 +7,6 @@ import Breadcrumbs from "metabase/components/Breadcrumbs"; import SidebarItem from "metabase/components/SidebarItem"; import cx from "classnames"; -import pure from "recompose/pure"; const SegmentFieldSidebar = ({ segment, field, style, className }) => (
    @@ -41,4 +40,4 @@ SegmentFieldSidebar.propTypes = { style: PropTypes.object, }; -export default pure(SegmentFieldSidebar); +export default React.memo(SegmentFieldSidebar); diff --git a/frontend/src/metabase/reference/segments/SegmentSidebar.jsx b/frontend/src/metabase/reference/segments/SegmentSidebar.jsx index 5c6e96717180..4210983f1ec9 100644 --- a/frontend/src/metabase/reference/segments/SegmentSidebar.jsx +++ b/frontend/src/metabase/reference/segments/SegmentSidebar.jsx @@ -3,7 +3,6 @@ import React from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; import cx from "classnames"; -import pure from "recompose/pure"; import MetabaseSettings from "metabase/lib/settings"; @@ -70,4 +69,4 @@ SegmentSidebar.propTypes = { style: PropTypes.object, }; -export default pure(SegmentSidebar); +export default React.memo(SegmentSidebar); diff --git a/frontend/src/metabase/selectors/metadata.js b/frontend/src/metabase/selectors/metadata.js index 96158bc80f3f..71853d8aaecf 100644 --- a/frontend/src/metabase/selectors/metadata.js +++ b/frontend/src/metabase/selectors/metadata.js @@ -13,7 +13,7 @@ import Metric from "metabase-lib/lib/metadata/Metric"; import Segment from "metabase-lib/lib/metadata/Segment"; import _ from "underscore"; -import { shallowEqual } from "recompose"; +import shallowEqual from "./shallowEqual"; import { getFieldValues, getRemappings } from "metabase/lib/query/field"; import { getIn } from "icepick"; diff --git a/frontend/src/metabase/selectors/shallowEqual.js b/frontend/src/metabase/selectors/shallowEqual.js new file mode 100644 index 000000000000..5e64178a8549 --- /dev/null +++ b/frontend/src/metabase/selectors/shallowEqual.js @@ -0,0 +1,76 @@ +/** + * NOTE: + * Copied directly from `https://github.com/acdlite/recompose/blob/master/src/packages/recompose/shallowEqual.js` + * as a temporary solution until we find an alternative to this utility. It was the only thing blocking the complete removal of `recompose` lib. + * + * Please see: https://github.com/metabase/metabase/pull/16829. + * + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @providesModule shallowEqual + * @typechecks + */ + +/* eslint-disable no-self-compare */ + +const hasOwnProperty = Object.prototype.hasOwnProperty; + +/** + * inlined Object.is polyfill to avoid requiring consumers ship their own + * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is + */ +function is(x, y) { + // SameValue algorithm + if (x === y) { + // Steps 1-5, 7-10 + // Steps 6.b-6.e: +0 != -0 + // Added the nonzero y check to make Flow happy, but it is redundant + return x !== 0 || y !== 0 || 1 / x === 1 / y; + } + // Step 6.a: NaN == NaN + return x !== x && y !== y; +} + +/** + * Performs equality by iterating through keys on an object and returning false + * when any key has values which are not strictly equal between the arguments. + * Returns true when the values of all keys are strictly equal. + */ +function shallowEqual(objA, objB) { + if (is(objA, objB)) { + return true; + } + + if ( + typeof objA !== "object" || + objA === null || + typeof objB !== "object" || + objB === null + ) { + return false; + } + + const keysA = Object.keys(objA); + const keysB = Object.keys(objB); + + if (keysA.length !== keysB.length) { + return false; + } + + // Test for A's keys different from B. + for (let i = 0; i < keysA.length; i++) { + if ( + !hasOwnProperty.call(objB, keysA[i]) || + !is(objA[keysA[i]], objB[keysA[i]]) + ) { + return false; + } + } + + return true; +} + +export default shallowEqual; diff --git a/package.json b/package.json index 682cc842fe18..b3cb6076f107 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,6 @@ "react-textarea-autosize": "^5.2.1", "react-transition-group": "1", "react-virtualized": "^9.7.2", - "recompose": "^0.30.0", "redux": "^3.5.2", "redux-actions": "^2.0.1", "redux-auth-wrapper": "^1.0.0", diff --git a/yarn.lock b/yarn.lock index 8080a8933fdd..45ce86a8740a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -822,13 +822,6 @@ core-js-pure "^3.0.0" regenerator-runtime "^0.13.4" -"@babel/runtime@^7.0.0": - version "7.14.0" - resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.14.0.tgz#46794bc20b612c5f75e62dd071e24dfd95f1cbe6" - integrity sha512-JELkvo/DlpNdJ7dlyw/eY7E0suy5i5GQH+Vlxaq1nsNJ+H7f4Vtv3jMeCEgRhZZQFXTjldYfQgv2qmM6M1v5wA== - dependencies: - regenerator-runtime "^0.13.4" - "@babel/runtime@^7.1.2", "@babel/runtime@^7.10.2", "@babel/runtime@^7.10.3", "@babel/runtime@^7.11.2", "@babel/runtime@^7.5.1", "@babel/runtime@^7.7.2", "@babel/runtime@^7.8.4", "@babel/runtime@^7.8.7": version "7.12.1" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.12.1.tgz#b4116a6b6711d010b2dad3b7b6e43bf1b9954740" @@ -3350,11 +3343,6 @@ chalk@^4.0.0, chalk@^4.1.0: ansi-styles "^4.1.0" supports-color "^7.1.0" -change-emitter@^0.1.2: - version "0.1.6" - resolved "https://registry.yarnpkg.com/change-emitter/-/change-emitter-0.1.6.tgz#e8b2fe3d7f1ab7d69a32199aff91ea6931409515" - integrity sha1-6LL+PX8at9aaMhma/5HqaTFAlRU= - character-entities-html4@^1.0.0: version "1.1.4" resolved "https://registry.yarnpkg.com/character-entities-html4/-/character-entities-html4-1.1.4.tgz#0e64b0a3753ddbf1fdc044c5fd01d0199a02e125" @@ -5751,7 +5739,7 @@ fb-watchman@^2.0.0: dependencies: bser "2.1.1" -fbjs@^0.8.1, fbjs@^0.8.16, fbjs@^0.8.9: +fbjs@^0.8.16, fbjs@^0.8.9: version "0.8.17" resolved "https://registry.yarnpkg.com/fbjs/-/fbjs-0.8.17.tgz#c4d598ead6949112653d6588b01a5cdcd9f90fdd" integrity sha1-xNWY6taUkRJlPWWIsBpc3Nn5D90= @@ -11205,7 +11193,7 @@ react-lazy-cache@^3.0.1: dependencies: deep-equal "^1.0.1" -react-lifecycles-compat@^3.0.0, react-lifecycles-compat@^3.0.2, react-lifecycles-compat@^3.0.4: +react-lifecycles-compat@^3.0.0, react-lifecycles-compat@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA== @@ -11490,18 +11478,6 @@ rechoir@^0.6.2: dependencies: resolve "^1.1.6" -recompose@^0.30.0: - version "0.30.0" - resolved "https://registry.yarnpkg.com/recompose/-/recompose-0.30.0.tgz#82773641b3927e8c7d24a0d87d65aeeba18aabd0" - integrity sha512-ZTrzzUDa9AqUIhRk4KmVFihH0rapdCSMFXjhHbNrjAWxBuUD/guYlyysMnuHjlZC/KRiOKRtB4jf96yYSkKE8w== - dependencies: - "@babel/runtime" "^7.0.0" - change-emitter "^0.1.2" - fbjs "^0.8.1" - hoist-non-react-statics "^2.3.1" - react-lifecycles-compat "^3.0.2" - symbol-observable "^1.0.4" - redent@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/redent/-/redent-1.0.0.tgz#cf916ab1fd5f1f16dfb20822dd6ec7f730c2afde" @@ -13103,7 +13079,7 @@ symbol-observable@1.0.1: resolved "https://registry.yarnpkg.com/symbol-observable/-/symbol-observable-1.0.1.tgz#8340fc4702c3122df5d22288f88283f513d3fdd4" integrity sha1-g0D8RwLDEi310iKI+IKD9RPT/dQ= -symbol-observable@^1.0.3, symbol-observable@^1.0.4, symbol-observable@^1.1.0, symbol-observable@^1.2.0: +symbol-observable@^1.0.3, symbol-observable@^1.1.0, symbol-observable@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/symbol-observable/-/symbol-observable-1.2.0.tgz#c22688aed4eab3cdc2dfeacbb561660560a00804" integrity sha512-e900nM8RRtGhlV36KGEU9k65K3mPb1WV70OdjfxlG2EAuM1noi/E/BaW/uMhL7bPEssK8QV57vN3esixjUvcXQ== From 3555b7b2d46e4aea6e2c7d449310735c94abb9a4 Mon Sep 17 00:00:00 2001 From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com> Date: Fri, 9 Jul 2021 13:11:16 +0200 Subject: [PATCH 002/664] Mark "Every field is hidden right now" string for translation (#16915) --- frontend/src/metabase/visualizations/visualizations/Table.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/metabase/visualizations/visualizations/Table.jsx b/frontend/src/metabase/visualizations/visualizations/Table.jsx index 654d1864e233..db1d8a15039b 100644 --- a/frontend/src/metabase/visualizations/visualizations/Table.jsx +++ b/frontend/src/metabase/visualizations/visualizations/Table.jsx @@ -417,7 +417,7 @@ export default class Table extends Component { " className="mb2" /> - Every field is hidden right now + {t`Every field is hidden right now`}
    ); } else { From 250118d2640d79e7706c8efd0a09aa46f6bd25f0 Mon Sep 17 00:00:00 2001 From: Anton Kulyk Date: Fri, 9 Jul 2021 14:34:55 +0300 Subject: [PATCH 003/664] Refactor CheckBox and StackedCheckBox components (#16913) * Disable react/prop-types for .info files * Refactor Checkbox * Refactor StackedCheckBox * Add a comment about Checkbox colors * Remove not used prop * Don't show outline for focused unchecked checkbox * Pass custom Checkbox label element * Refactor EmailAttachmentPicker to use label * Refactor FieldsPicker to use label * Refactor FilterOptions to use label * Refactor DisplayOptionsPane to use label * Refactor ScratchApp to use label * Refactor AddSeriesModal to use label * Add "danger" color to colors * Refactor DeleteModalWithConfirm to use label * Add label usage examples for CheckBox * Add label usage examples for StackedCheckBox * Add disabled state to CheckBox * Add disabled state to StackedCheckBox * Fix label prop-types * Add space between examples * Fix StackedCheckBox layout * Add. StackedCheckBox label * Fix disabled state for StackedCheckBox * Migrate FieldsPicker to use label * Migrate EmailAttachmentPicker to use label * Add .styled files for checkbox components * Fix e2e tests * Fix `uncheckedColor` passed to HTML elements * Revert eslint overwrite for .info files * Display focus outline --- .../src/metabase/components/CheckBox.info.js | 55 ++++- frontend/src/metabase/components/CheckBox.jsx | 205 +++++++++++------- .../metabase/components/CheckBox.styled.js | 78 +++++++ .../components/DeleteModalWithConfirm.jsx | 34 +-- .../DeleteModalWithConfirm.styled.js | 9 + .../components/StackedCheckBox.info.js | 55 ++++- .../metabase/components/StackedCheckBox.jsx | 84 +++++-- .../components/StackedCheckBox.styled.js | 36 +++ .../dashboard/components/AddSeriesModal.jsx | 2 +- .../internal/components/ScratchApp.jsx | 2 +- frontend/src/metabase/lib/colors.js | 1 + .../components/widgets/DisplayOptionsPane.jsx | 4 +- .../components/filters/FilterOptions.jsx | 10 +- .../filters/pickers/SelectPicker.jsx | 2 +- .../notebook/steps/FieldsPicker.jsx | 43 ++-- .../components/EmailAttachmentPicker.jsx | 19 +- .../collections/collections.cy.spec.js | 18 +- .../scenarios/dashboard/dashboard.cy.spec.js | 2 +- .../sharing/subscriptions.cy.spec.js | 2 +- 19 files changed, 475 insertions(+), 186 deletions(-) create mode 100644 frontend/src/metabase/components/CheckBox.styled.js create mode 100644 frontend/src/metabase/components/DeleteModalWithConfirm.styled.js create mode 100644 frontend/src/metabase/components/StackedCheckBox.styled.js diff --git a/frontend/src/metabase/components/CheckBox.info.js b/frontend/src/metabase/components/CheckBox.info.js index 40a5b4e2a404..9682495403de 100644 --- a/frontend/src/metabase/components/CheckBox.info.js +++ b/frontend/src/metabase/components/CheckBox.info.js @@ -1,4 +1,5 @@ -import React from "react"; +/* eslint-disable react/prop-types */ +import React, { useState } from "react"; import CheckBox from "metabase/components/CheckBox"; export const component = CheckBox; @@ -8,11 +9,51 @@ export const description = ` A standard checkbox. `; +function CheckBoxDemo({ checked: isCheckedInitially = false, ...props } = {}) { + const [checked, setChecked] = useState(isCheckedInitially); + return ( + setChecked(e.target.checked)} + /> + ); +} + +const rowStyle = { + display: "flex", + alignItems: "center", + justifyContent: "space-around", +}; + export const examples = { - "Default - Off": , - "On - Default blue": , - Purple: , - Yellow: , - Red: , - Green: , + Default: , + Label: ( +
    + + Custom element label} + /> +
    + ), + Disabled: ( +
    + + +
    + ), + Sizing: ( +
    + {[10, 12, 14, 16, 18, 20, 24].map(size => ( + + ))} +
    + ), + Colors: ( +
    + {["accent1", "accent2", "accent3", "accent4"].map(color => ( + + ))} +
    + ), }; diff --git a/frontend/src/metabase/components/CheckBox.jsx b/frontend/src/metabase/components/CheckBox.jsx index eeecd3b1dc5a..b61ee0a29d3d 100644 --- a/frontend/src/metabase/components/CheckBox.jsx +++ b/frontend/src/metabase/components/CheckBox.jsx @@ -1,93 +1,136 @@ -/* eslint-disable react/prop-types */ -import React, { Component } from "react"; +import React, { useCallback, useState } from "react"; import PropTypes from "prop-types"; -import cx from "classnames"; +import { + CheckboxRoot, + Container, + VisibleBox, + Input, + CheckboxIcon, + LabelText, +} from "./CheckBox.styled"; -import Icon from "metabase/components/Icon"; +const propTypes = { + checked: PropTypes.bool, + indeterminate: PropTypes.bool, + label: PropTypes.node, + disabled: PropTypes.bool, + onChange: PropTypes.func, + onFocus: PropTypes.func, + onBlur: PropTypes.func, -import { color as c, normal as defaultColors } from "metabase/lib/colors"; -import { KEYCODE_SPACE } from "metabase/lib/keyboard"; + // Expect color aliases, literals + // Example: brand, accent1, success + // Won't work: red, #000, rgb(0, 0, 0) + checkedColor: PropTypes.string, + uncheckedColor: PropTypes.string, -export default class CheckBox extends Component { - static propTypes = { - checked: PropTypes.bool, - indeterminate: PropTypes.bool, - onChange: PropTypes.func, - color: PropTypes.oneOf(Object.keys(defaultColors)), - size: PropTypes.number, // TODO - this should probably be a concrete set of options - padding: PropTypes.number, // TODO - the component should pad itself properly based on the size - noIcon: PropTypes.bool, - }; + size: PropTypes.number, + autoFocus: PropTypes.bool, - static defaultProps = { - size: 16, - padding: 2, - color: "blue", - style: {}, - }; + className: PropTypes.string, +}; - onClick(e) { - if (this.props.onChange) { - // TODO: use a proper event object? - this.props.onChange({ - // add preventDefault so checkboxes can optionally prevent - preventDefault: () => e.preventDefault(), - target: { checked: !this.props.checked }, - }); - } - } +export const DEFAULT_CHECKED_COLOR = "brand"; +export const DEFAULT_UNCHECKED_COLOR = "text-light"; +export const DEFAULT_SIZE = 16; - onKeyPress = e => { - if (e.keyCode === KEYCODE_SPACE) { - this.onClick(e); - } - }; +const ICON_PADDING = 4; + +function Checkbox({ + label, + checked, + indeterminate, + disabled = false, + onChange, + onFocus, + onBlur, + checkedColor = DEFAULT_CHECKED_COLOR, + uncheckedColor = DEFAULT_UNCHECKED_COLOR, + size = DEFAULT_SIZE, + autoFocus = false, + className, + ...props +}) { + const [isFocused, setFocused] = useState(autoFocus); + + const handleFocus = useCallback( + e => { + setFocused(true); + if (typeof onFocus === "function") { + onFocus(e); + } + }, + [onFocus], + ); - render() { - const { - className, - style, - checked, - indeterminate, - color, - padding, - size, - noIcon, - } = this.props; + const handleBlur = useCallback( + e => { + setFocused(false); + if (typeof onBlur === "function") { + onBlur(e); + } + }, + [onBlur], + ); - const checkedColor = defaultColors[color]; - const uncheckedColor = c("text-light"); + const onKeyPress = useCallback( + e => { + if (e.key === "Enter" && typeof onChange === "function") { + onChange({ + preventDefault: () => e.preventDefault(), + target: { checked: !checked }, + }); + } + }, + [checked, onChange], + ); - const checkboxStyle = { - width: size, - height: size, - backgroundColor: checked ? checkedColor : "white", - border: `2px solid ${checked ? checkedColor : uncheckedColor}`, - borderRadius: 4, - }; - return ( -
    { - this.onClick(e); - }} - onKeyPress={this.onKeyPress} - role="checkbox" - aria-checked={checked} - tabIndex="0" - > - {(checked || indeterminate) && !noIcon && ( - { + if (label == null) { + return null; + } + return React.isValidElement(label) ? label : {label}; + }, [label]); + + return ( + + + + - )} -
    - ); - } + {(checked || indeterminate) && ( + + )} + + {renderLabel()} + + + ); } + +Checkbox.propTypes = propTypes; +Checkbox.Label = LabelText; + +export default Checkbox; diff --git a/frontend/src/metabase/components/CheckBox.styled.js b/frontend/src/metabase/components/CheckBox.styled.js new file mode 100644 index 000000000000..f88fabca5e4a --- /dev/null +++ b/frontend/src/metabase/components/CheckBox.styled.js @@ -0,0 +1,78 @@ +import React from "react"; +import styled, { css } from "styled-components"; +import _ from "underscore"; +import Icon from "metabase/components/Icon"; +import { color, darken } from "metabase/lib/colors"; + +export const CheckboxRoot = styled.label` + display: block; + cursor: pointer; + + ${props => + props.disabled && + css` + opacity: 0.4; + pointer-events: none; + `} +`; + +export const Container = styled.div` + display: flex; + align-items: center; +`; + +export const VisibleBox = styled.span` + display: flex; + align-items: center; + justify-center: center; + position: relative; + width: ${props => `${props.size}px`}; + height: ${props => `${props.size}px`}; + + background-color: ${props => + props.checked ? color(props.checkedColor) : color("bg-white")}; + + border: 2px solid + ${props => + props.checked ? color(props.checkedColor) : color(props.uncheckedColor)}; + + border-radius: 4px; + + ${props => + props.isFocused && + css` + outline: 1px auto + ${props.checked + ? darken(color(props.checkedColor)) + : color(props.checkedColor)}; + outline-offset: 1px; + `} +`; + +export const Input = styled.input.attrs({ type: "checkbox" })` + cursor: inherit; + position: absolute; + opacity: 0; + width: 100%; + height: 100%; + top: 0; + left: 0; + margin: 0; + padding: 0; + z-index: 1; +`; + +function IconWrapped(props) { + const iconProps = _.omit(props, "uncheckedColor"); + return ; +} + +export const CheckboxIcon = styled(IconWrapped)` + position: absolute; + color: ${props => + props.checked ? color("white") : color(props.uncheckedColor)}; +`; + +export const LabelText = styled.span` + margin-left: 8px; +`; diff --git a/frontend/src/metabase/components/DeleteModalWithConfirm.jsx b/frontend/src/metabase/components/DeleteModalWithConfirm.jsx index faa569a6ae82..5b033093066f 100644 --- a/frontend/src/metabase/components/DeleteModalWithConfirm.jsx +++ b/frontend/src/metabase/components/DeleteModalWithConfirm.jsx @@ -1,12 +1,14 @@ import React, { Component } from "react"; import PropTypes from "prop-types"; - -import ModalContent from "metabase/components/ModalContent"; -import CheckBox from "metabase/components/CheckBox"; import { t } from "ttag"; import cx from "classnames"; import _ from "underscore"; +import ModalContent from "metabase/components/ModalContent"; +import CheckBox from "metabase/components/CheckBox"; + +import { CheckboxLabel } from "./DeleteModalWithConfirm.styled"; + export default class DeleteModalWithConfirm extends Component { constructor(props, context) { super(props, context); @@ -47,20 +49,18 @@ export default class DeleteModalWithConfirm extends Component { key={index} className="pb2 mb2 border-row-divider flex align-center" > - - - this.setState({ - checked: { ...checked, [index]: e.target.checked }, - }) - } - /> - - {item} + {item}} + size={20} + checkedColor="danger" + uncheckedColor="danger" + checked={checked[index]} + onChange={e => + this.setState({ + checked: { ...checked, [index]: e.target.checked }, + }) + } + />
    • ))} diff --git a/frontend/src/metabase/components/DeleteModalWithConfirm.styled.js b/frontend/src/metabase/components/DeleteModalWithConfirm.styled.js new file mode 100644 index 000000000000..f4d6b30f1fd8 --- /dev/null +++ b/frontend/src/metabase/components/DeleteModalWithConfirm.styled.js @@ -0,0 +1,9 @@ +import styled from "styled-components"; +import CheckBox from "metabase/components/CheckBox"; + +import { color } from "metabase/lib/colors"; + +export const CheckboxLabel = styled(CheckBox.Label)` + color: ${color("danger")}; + font-size: 1.12em; +`; diff --git a/frontend/src/metabase/components/StackedCheckBox.info.js b/frontend/src/metabase/components/StackedCheckBox.info.js index 03dc6d27245a..36ca86bf5f32 100644 --- a/frontend/src/metabase/components/StackedCheckBox.info.js +++ b/frontend/src/metabase/components/StackedCheckBox.info.js @@ -1,4 +1,5 @@ -import React from "react"; +/* eslint-disable react/prop-types */ +import React, { useState } from "react"; import StackedCheckBox from "metabase/components/StackedCheckBox"; export const component = StackedCheckBox; @@ -8,8 +9,54 @@ export const description = ` A stacked checkbox, representing "all" items. `; +function StackedCheckBoxDemo({ + checked: isCheckedInitially = false, + ...props +}) { + const [checked, setChecked] = useState(isCheckedInitially); + return ( + setChecked(e.target.checked)} + /> + ); +} + +const rowStyle = { + display: "flex", + alignItems: "center", + justifyContent: "space-around", +}; + export const examples = { - "Off - Default": , - Checked: , - "Checked with color": , + Default: , + Label: ( +
    + +
    + Custom element label} /> +
    + ), + Disabled: ( +
    + +
    + +
    + ), + Sizing: ( +
    + {[10, 12, 14, 16, 18, 20, 24].map(size => ( + + ))} +
    + ), + Colors: ( +
    + {["accent1", "accent2", "accent3", "accent4"].map(color => ( + + ))} +
    + ), }; diff --git a/frontend/src/metabase/components/StackedCheckBox.jsx b/frontend/src/metabase/components/StackedCheckBox.jsx index 85c514196877..f6f2db3d8c49 100644 --- a/frontend/src/metabase/components/StackedCheckBox.jsx +++ b/frontend/src/metabase/components/StackedCheckBox.jsx @@ -1,25 +1,69 @@ -/* eslint-disable react/prop-types */ -import React from "react"; -import cx from "classnames"; +import React, { useCallback } from "react"; +import PropTypes from "prop-types"; -import CheckBox from "metabase/components/CheckBox"; +import { + DEFAULT_CHECKED_COLOR, + DEFAULT_UNCHECKED_COLOR, + DEFAULT_SIZE, +} from "metabase/components/CheckBox"; -const OFFSET = 3; +import { + StackedCheckBoxRoot, + OpaqueCheckBox, + StackedBackground, + Label, +} from "./StackedCheckBox.styled"; -const StackedCheckBox = ({ className, ...props }) => ( -
    - - -
    -); +const propTypes = { + label: PropTypes.node, + checked: PropTypes.bool, + disabled: PropTypes.bool, + checkedColor: PropTypes.string, + uncheckedColor: PropTypes.string, + size: PropTypes.number, + className: PropTypes.string, +}; + +function StackedCheckBox({ + label, + checked, + disabled = false, + checkedColor = DEFAULT_CHECKED_COLOR, + uncheckedColor = DEFAULT_UNCHECKED_COLOR, + size = DEFAULT_SIZE, + className, + ...props +}) { + const renderLabel = useCallback(() => { + if (label == null) { + return null; + } + return ; + }, [label]); + + return ( + + + + + ); +} + +StackedCheckBox.propTypes = propTypes; +StackedCheckBox.Label = Label; export default StackedCheckBox; diff --git a/frontend/src/metabase/components/StackedCheckBox.styled.js b/frontend/src/metabase/components/StackedCheckBox.styled.js new file mode 100644 index 000000000000..d01b2d8948ef --- /dev/null +++ b/frontend/src/metabase/components/StackedCheckBox.styled.js @@ -0,0 +1,36 @@ +import styled from "styled-components"; +import CheckBox from "metabase/components/CheckBox"; +import { color } from "metabase/lib/colors"; + +export const StackedCheckBoxRoot = styled.div` + position: relative; + transform: scale(1); + opacity: ${props => (props.disabled ? 0.4 : 1)}; +`; + +export const OpaqueCheckBox = styled(CheckBox)` + opacity: 1; +`; + +export const StackedBackground = styled.div` + width: ${props => `${props.size}px`}; + height: ${props => `${props.size}px`}; + border-radius: 4px; + position: absolute; + display: inline-block; + + z-index: -1; + top: -3px; + left: 3px; + + background: ${props => + props.checked ? color(props.checkedColor) : color("bg-white")}; + + border: 2px solid + ${props => + props.checked ? color(props.checkedColor) : color(props.uncheckedColor)}; +`; + +export const Label = styled(CheckBox.Label)` + margin-top: -2px; +`; diff --git a/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx b/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx index fc51d5c4c7da..ec1a4a30d4b6 100644 --- a/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx +++ b/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx @@ -329,6 +329,7 @@ export default class AddSeriesModal extends Component { > this.handleQuestionSelectedChange( @@ -338,7 +339,6 @@ export default class AddSeriesModal extends Component { } /> - {question.displayName()} {!question.isStructured() && (
    - Centered: this.setState({ centered: !centered })} /> diff --git a/frontend/src/metabase/lib/colors.js b/frontend/src/metabase/lib/colors.js index 8183a25a731f..b50698a06c87 100644 --- a/frontend/src/metabase/lib/colors.js +++ b/frontend/src/metabase/lib/colors.js @@ -25,6 +25,7 @@ const colors = { white: "#FFFFFF", black: "#2E353B", success: "#84BB4C", + danger: "#ED6E6E", error: "#ED6E6E", warning: "#F9CF48", "text-dark": "#4C5773", diff --git a/frontend/src/metabase/public/components/widgets/DisplayOptionsPane.jsx b/frontend/src/metabase/public/components/widgets/DisplayOptionsPane.jsx index ccf14cac8fc4..e21f1ca298e2 100644 --- a/frontend/src/metabase/public/components/widgets/DisplayOptionsPane.jsx +++ b/frontend/src/metabase/public/components/widgets/DisplayOptionsPane.jsx @@ -24,6 +24,7 @@ const DisplayOptionsPane = ({
    onChangeDisplayOptions({ @@ -32,10 +33,10 @@ const DisplayOptionsPane = ({ }) } /> - {t`Border`}
    onChangeDisplayOptions({ @@ -44,7 +45,6 @@ const DisplayOptionsPane = ({ }) } /> - {t`Title`}
    {options.map(([name, option]) => ( -
    this.toggleOptionValue(name)} - > +
    this.toggleOptionValue(name)} /> -
    ))}
    diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/SelectPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/SelectPicker.jsx index c223af56518c..5d57ca01dc10 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/SelectPicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/SelectPicker.jsx @@ -137,7 +137,7 @@ export default class SelectPicker extends Component { >

    {this.nameForOption(option)}

    diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/FieldsPicker.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/FieldsPicker.jsx index 24620a81d251..871181f3bf69 100644 --- a/frontend/src/metabase/query_builder/components/notebook/steps/FieldsPicker.jsx +++ b/frontend/src/metabase/query_builder/components/notebook/steps/FieldsPicker.jsx @@ -2,7 +2,6 @@ import React from "react"; import { t } from "ttag"; -import cx from "classnames"; import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; import CheckBox from "metabase/components/CheckBox"; @@ -27,37 +26,33 @@ export default function FieldsPicker({ >
      {(onSelectAll || onSelectNone) && ( -
    • { - if (isAll) { - onSelectNone(); - } else { - onSelectAll(); - } - }} - > +
    • { + if (isAll) { + onSelectNone(); + } else { + onSelectAll(); + } + }} className="mr1" /> - {isAll && onSelectNone ? t`Select None` : t`Select All`}
      • )} {dimensions.map(dimension => ( -
    • { - onToggleDimension(dimension, !selected.has(dimension.key())); - }} - > - - {dimension.displayName()} +
    • + { + onToggleDimension(dimension, !selected.has(dimension.key())); + }} + className="mr1" + />
      • ))}
    diff --git a/frontend/src/metabase/sharing/components/EmailAttachmentPicker.jsx b/frontend/src/metabase/sharing/components/EmailAttachmentPicker.jsx index 8b746684a961..281e644a36d6 100644 --- a/frontend/src/metabase/sharing/components/EmailAttachmentPicker.jsx +++ b/frontend/src/metabase/sharing/components/EmailAttachmentPicker.jsx @@ -6,7 +6,6 @@ import { t } from "ttag"; import ButtonGroup from "metabase/components/ButtonGroup"; import CheckBox from "metabase/components/CheckBox"; -import Text from "metabase/components/type/Text"; import Label from "metabase/components/type/Label"; import StackedCheckBox from "metabase/components/StackedCheckBox"; import Toggle from "metabase/components/Toggle"; @@ -190,33 +189,31 @@ export default class EmailAttachmentPicker extends Component { />
    -
      -
    • +
        +
      • - {t`Questions to attach`}
        • {cards.map(card => (
      • { - this.onToggleCard(card); - }} > { + this.onToggleCard(card); + }} className="mr1" /> - {card.name}
        • ))}
      diff --git a/frontend/test/metabase/scenarios/collections/collections.cy.spec.js b/frontend/test/metabase/scenarios/collections/collections.cy.spec.js index b58df71f4a95..a0f9bfe95be5 100644 --- a/frontend/test/metabase/scenarios/collections/collections.cy.spec.js +++ b/frontend/test/metabase/scenarios/collections/collections.cy.spec.js @@ -526,14 +526,15 @@ describe("scenarios > collection_defaults", () => { selectItemUsingCheckbox("Orders"); cy.findByText("1 item selected").should("be.visible"); - // Select all - cy.icon("dash").click(); - cy.icon("dash").should("not.exist"); - cy.findByText("4 items selected"); - - // Deselect all cy.findByTestId("bulk-action-bar").within(() => { - cy.icon("check").click(); + // Select all + cy.findByTestId("checkbox-root").should("be.visible"); + cy.icon("dash").click({ force: true }); + cy.icon("dash").should("not.exist"); + cy.findByText("4 items selected"); + + // Deselect all + cy.icon("check").click({ force: true }); }); cy.icon("check").should("not.exist"); cy.findByTestId("bulk-action-bar").should("not.be.visible"); @@ -630,8 +631,9 @@ function selectItemUsingCheckbox(item, icon = "table") { .closest("tr") .within(() => { cy.icon(icon).trigger("mouseover"); - cy.findByRole("checkbox") + cy.findByTestId("checkbox-root") .should("be.visible") + .findByRole("checkbox") .click(); }); } diff --git a/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js index 9c202ecca3bc..07f2eacdc3ef 100644 --- a/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js +++ b/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js @@ -603,7 +603,7 @@ describe("scenarios > dashboard", () => { // which is actually "Include" followed by "this minute" wrapped in , so has to be clicked this way cy.contains("Include this minute").click(); // make sure the checkbox was checked - cy.findByRole("checkbox").should("have.attr", "aria-checked", "true"); + cy.findByRole("checkbox").should("be.checked"); }); it("user without data permissions should be able to use dashboard filter (metabase#15119)", () => { diff --git a/frontend/test/metabase/scenarios/sharing/subscriptions.cy.spec.js b/frontend/test/metabase/scenarios/sharing/subscriptions.cy.spec.js index ea2d6d161d19..6378f49f7e9d 100644 --- a/frontend/test/metabase/scenarios/sharing/subscriptions.cy.spec.js +++ b/frontend/test/metabase/scenarios/sharing/subscriptions.cy.spec.js @@ -110,7 +110,7 @@ describe("scenarios > dashboard > subscriptions", () => { cy.findAllByRole("listitem") .contains("Orders") // yields the whole
    • element .within(() => { - cy.findByRole("checkbox").should("have.attr", "aria-checked", "true"); + cy.findByRole("checkbox").should("be.checked"); }); }); From c04e9a63f5614564f078e2b36e393747009fba9b Mon Sep 17 00:00:00 2001 From: dpsutton Date: Fri, 9 Jul 2021 15:29:48 -0500 Subject: [PATCH 004/664] Render/scalar (#16955) * Scalar rendering as text in slack * Fix tests for pulse * Tests for slack scalars --- src/metabase/pulse.clj | 50 +++++---- src/metabase/pulse/render.clj | 42 +++++--- src/metabase/pulse/render/body.clj | 12 ++- src/metabase/pulse/render/common.clj | 5 +- test/metabase/dashboard_subscription_test.clj | 14 +-- test/metabase/pulse/render/body_test.clj | 76 +++++++------ test/metabase/pulse/test_util.clj | 13 +-- test/metabase/pulse_test.clj | 102 +++++++++++++----- 8 files changed, 197 insertions(+), 117 deletions(-) diff --git a/src/metabase/pulse.clj b/src/metabase/pulse.clj index a335c1de2813..1ed42b3b4a7d 100644 --- a/src/metabase/pulse.clj +++ b/src/metabase/pulse.clj @@ -56,13 +56,13 @@ (merge {:executed-by pulse-creator-id :context :pulse :card-id card-id} - options))))] - (let [result (if pulse-creator-id - (session/with-current-user pulse-creator-id - (process-query)) - (process-query))] - {:card card - :result result}))) + options)))) + result (if pulse-creator-id + (session/with-current-user pulse-creator-id + (process-query)) + (process-query))] + {:card card + :result result})) (catch Throwable e (log/warn e (trs "Error running query for Card {0}" card-id)))))) @@ -123,22 +123,30 @@ [card-results] (let [{channel-id :id} (slack/files-channel)] (for [{{card-id :id, card-name :name, :as card} :card, result :result} card-results] - {:title card-name - :attachment-bytes-thunk (fn [] (render/render-pulse-card-to-png (defaulted-timezone card) card result)) - :title_link (urls/card-url card-id) - :attachment-name "image.png" - :channel-id channel-id - :fallback card-name}))) + {:title card-name + :rendered-info (render/render-pulse-card :inline (defaulted-timezone card) card result) + :title_link (urls/card-url card-id) + :attachment-name "image.png" + :channel-id channel-id + :fallback card-name}))) (defn create-and-upload-slack-attachments! - "Create an attachment in Slack for a given Card by rendering its result into an image and uploading it." - [attachments] - (doall - (for [{:keys [attachment-bytes-thunk attachment-name channel-id] :as attachment-data} attachments] - (let [slack-file-url (slack/upload-file! (attachment-bytes-thunk) attachment-name channel-id)] - (-> attachment-data - (select-keys [:title :title_link :fallback]) - (assoc :image_url slack-file-url)))))) + "Create an attachment in Slack for a given Card by rendering its result into an image and uploading + it. Slack-attachment-uploader is a function which takes image-bytes and an attachment name, uploads the file, and + returns an image url, defaulting to slack/upload-file!." + ([attachments] (create-and-upload-slack-attachments! attachments slack/upload-file!)) + ([attachments slack-attachment-uploader] + (letfn [(f [a] (select-keys a [:title :title_link :fallback]))] + (reduce (fn [processed {:keys [rendered-info attachment-name channel-id] :as attachment-data}] + (conj processed (if (:render/text rendered-info) + (-> (f attachment-data) + (assoc :text (:render/text rendered-info))) + (let [image-bytes (render/png-from-render-info rendered-info) + image-url (slack-attachment-uploader image-bytes attachment-name channel-id)] + (-> (f attachment-data) + (assoc :image_url image-url)))))) + [] + attachments)))) (defn- is-card-empty? "Check if the card is empty" diff --git a/src/metabase/pulse/render.clj b/src/metabase/pulse/render.clj index 8197fb6a617d..3a47353a4821 100644 --- a/src/metabase/pulse/render.clj +++ b/src/metabase/pulse/render.clj @@ -109,22 +109,30 @@ [card] (h (urls/card-url (:id card)))) -(s/defn ^:private render-pulse-card :- common/RenderedPulseCard - "Render a single `card` for a `Pulse` to Hiccup HTML. `result` is the QP results." +(s/defn render-pulse-card :- common/RenderedPulseCard + "Render a single `card` for a `Pulse` to Hiccup HTML. `result` is the QP results. Returns a map with keys + +- attachments +- content (a hiccup form suitable for rending on rich clients or rendering into an image) +- render/text : raw text suitable for substituting on clients when text is preferable. (Currently slack uses this for + scalar results where text is preferable to an image of a div of a single result." [render-type timezone-id :- (s/maybe s/Str) card results] - (let [{title :content, title-attachments :attachments} (make-title-if-needed render-type card) - {pulse-body :content, body-attachments :attachments} (render-pulse-card-body render-type timezone-id card results)] - {:attachments (merge title-attachments body-attachments) - :content [:a {:href (card-href card) - :target "_blank" - :style (style/style - (style/section-style) - {:margin :16px - :margin-bottom :16px - :display :block - :text-decoration :none})} - title - pulse-body]})) + (let [{title :content, title-attachments :attachments} (make-title-if-needed render-type card) + {pulse-body :content + body-attachments :attachments + text :render/text} (render-pulse-card-body render-type timezone-id card results)] + (cond-> {:attachments (merge title-attachments body-attachments) + :content [:a {:href (card-href card) + :target "_blank" + :style (style/style + (style/section-style) + {:margin :16px + :margin-bottom :16px + :display :block + :text-decoration :none})} + title + pulse-body]} + text (assoc :render/text text)))) (defn render-pulse-card-for-display "Same as `render-pulse-card` but isn't intended for an email, rather for previewing so there is no need for @@ -155,3 +163,7 @@ "Render a `pulse-card` as a PNG. `data` is the `:data` from a QP result (I think...)" [timezone-id :- (s/maybe s/Str) pulse-card result] (png/render-html-to-png (render-pulse-card :inline timezone-id pulse-card result) card-width)) + +(s/defn png-from-render-info :- bytes + [rendered-info :- common/RenderedPulseCard] + (png/render-html-to-png rendered-info card-width)) diff --git a/src/metabase/pulse/render/body.clj b/src/metabase/pulse/render/body.clj index 1a56ca85e9bd..7dae62f7aee9 100644 --- a/src/metabase/pulse/render/body.clj +++ b/src/metabase/pulse/render/body.clj @@ -220,12 +220,14 @@ (s/defmethod render :scalar :- common/RenderedPulseCard [_ _ timezone-id card {:keys [cols rows]}] - {:attachments - nil + (let [value (format-cell timezone-id (ffirst rows) (first cols))] + {:attachments + nil - :content - [:div {:style (style/style (style/scalar-style))} - (h (format-cell timezone-id (ffirst rows) (first cols)))]}) + :content + [:div {:style (style/style (style/scalar-style))} + (h value)] + :render/text (str value)})) (s/defmethod render :sparkline :- common/RenderedPulseCard [_ render-type timezone-id card {:keys [rows cols] :as data}] diff --git a/src/metabase/pulse/render/common.clj b/src/metabase/pulse/render/common.clj index 8ba4b4b66978..4e48ce94a0fb 100644 --- a/src/metabase/pulse/render/common.clj +++ b/src/metabase/pulse/render/common.clj @@ -11,8 +11,9 @@ (def RenderedPulseCard "Schema used for functions that operate on pulse card contents and their attachments" - {:attachments (s/maybe {s/Str URL}) - :content [s/Any]}) + {:attachments (s/maybe {s/Str URL}) + :content [s/Any] + (s/optional-key :render/text) (s/maybe s/Str)}) (p.types/defrecord+ NumericWrapper [num-str] hutil/ToString diff --git a/test/metabase/dashboard_subscription_test.clj b/test/metabase/dashboard_subscription_test.clj index 63f4dabf1055..1b0c7bf381e9 100644 --- a/test/metabase/dashboard_subscription_test.clj +++ b/test/metabase/dashboard_subscription_test.clj @@ -170,18 +170,18 @@ (fn [{:keys [card-id]} [pulse-results]] ;; If we don't force the thunk, the rendering code will never execute and attached-results-text won't be ;; called - (force-bytes-thunk pulse-results) (testing "\"more results in attachment\" text should not be present for Slack Pulses" (testing "Pulse results" (is (= {:channel-id "#general" :message "Aviary KPIs" :attachments - [{:title card-name - :attachment-bytes-thunk true - :title_link (str "https://metabase.com/testmb/question/" card-id) - :attachment-name "image.png" - :channel-id "FOO" - :fallback card-name}]} + [{:title card-name + :rendered-info {:attachments false + :content true} + :title_link (str "https://metabase.com/testmb/question/" card-id) + :attachment-name "image.png" + :channel-id "FOO" + :fallback card-name}]} (thunk->boolean pulse-results)))) (testing "attached-results-text should be invoked exactly once" (is (= 1 diff --git a/test/metabase/pulse/render/body_test.clj b/test/metabase/pulse/render/body_test.clj index f494bc23db94..3f5743a11adf 100644 --- a/test/metabase/pulse/render/body_test.clj +++ b/test/metabase/pulse/render/body_test.clj @@ -4,7 +4,8 @@ [hiccup.core :refer [html]] [metabase.pulse.render.body :as body] [metabase.pulse.render.common :as common] - [metabase.pulse.render.test-util :as render.tu])) + [metabase.pulse.render.test-util :as render.tu] + [schema.core :as s])) (def ^:private pacific-tz "America/Los_Angeles") @@ -248,36 +249,49 @@ :content last)) -(deftest renders-int - (is (= "10" - (render-scalar-value {:cols [{:name "ID", - :display_name "ID", - :base_type :type/BigInteger - :semantic_type nil}] - :rows [[10]]})))) - -(deftest renders-float - (is (= "10.12" - (render-scalar-value {:cols [{:name "floatnum", - :display_name "FLOATNUM", - :base_type :type/Float - :semantic_type nil}] - :rows [[10.12345]]})))) - -(deftest renders-string - (is (= "foo" - (render-scalar-value {:cols [{:name "stringvalue", - :display_name "STRINGVALUE", - :base_type :type/Text - :semantic_type nil}] - :rows [["foo"]]})))) -(deftest renders-date - (is (= "Apr 1, 2014" - (render-scalar-value {:cols [{:name "date", - :display_name "DATE", - :base_type :type/DateTime - :semantic_type nil}] - :rows [["2014-04-01T08:30:00.0000"]]})))) +(deftest scalar-test + (testing "renders int" + (is (= "10" + (render-scalar-value {:cols [{:name "ID", + :display_name "ID", + :base_type :type/BigInteger + :semantic_type nil}] + :rows [[10]]})))) + (testing "renders float" + (is (= "10.12" + (render-scalar-value {:cols [{:name "floatnum", + :display_name "FLOATNUM", + :base_type :type/Float + :semantic_type nil}] + :rows [[10.12345]]})))) + (testing "renders string" + (is (= "foo" + (render-scalar-value {:cols [{:name "stringvalue", + :display_name "STRINGVALUE", + :base_type :type/Text + :semantic_type nil}] + :rows [["foo"]]})))) + (testing "renders date" + (is (= "Apr 1, 2014" + (render-scalar-value {:cols [{:name "date", + :display_name "DATE", + :base_type :type/DateTime + :semantic_type nil}] + :rows [["2014-04-01T08:30:00.0000"]]})))) + (testing "Includes raw text" + (let [results {:cols [{:name "stringvalue", + :display_name "STRINGVALUE", + :base_type :type/Text + :semantic_type nil}] + :rows [["foo"]]}] + (is (= "foo" + (:render/text (body/render :scalar nil pacific-tz nil results)))) + (is (schema= {:attachments (s/eq nil) + :content [(s/one (s/eq :div) "div tag") + (s/one {:style s/Str} "style map") + (s/one (s/eq "foo") "content")] + :render/text (s/eq "foo")} + (body/render :scalar nil pacific-tz nil results)))))) (defn- replace-style-maps [hiccup-map] (walk/postwalk (fn [maybe-map] diff --git a/test/metabase/pulse/test_util.clj b/test/metabase/pulse/test_util.clj index 58a8292c8075..485738a1072d 100644 --- a/test/metabase/pulse/test_util.clj +++ b/test/metabase/pulse/test_util.clj @@ -1,5 +1,6 @@ (ns metabase.pulse.test-util (:require [clojure.walk :as walk] + [medley.core :as m] [metabase.integrations.slack :as slack] [metabase.models.pulse :as models.pulse :refer [Pulse]] [metabase.models.pulse-card :refer [PulseCard]] @@ -132,14 +133,8 @@ (invoke [_ x1 x2 x3 x4 x5 x6] (invoke-with-wrapping input output func [x1 x2 x3 x4 x5 x6]))))) -(defn force-bytes-thunk - "Grabs the thunk that produces the image byte array and invokes it" - [results] - ((-> results - :attachments - first - :attachment-bytes-thunk))) - (defn thunk->boolean [{:keys [attachments] :as result}] (assoc result :attachments (for [attachment-info attachments] - (update attachment-info :attachment-bytes-thunk fn?)))) + (-> attachment-info + (update :rendered-info (fn [ri] + (m/map-vals some? ri))))))) diff --git a/test/metabase/pulse_test.clj b/test/metabase/pulse_test.clj index 54b9e9874486..986f71a452bb 100644 --- a/test/metabase/pulse_test.clj +++ b/test/metabase/pulse_test.clj @@ -8,6 +8,7 @@ [metabase.models.permissions-group :as group] [metabase.models.pulse :as models.pulse] [metabase.pulse :as pulse] + [metabase.pulse.render :as render] [metabase.pulse.render.body :as render.body] [metabase.pulse.test-util :refer :all] [metabase.query-processor.middleware.constraints :as constraints] @@ -133,8 +134,10 @@ (def ^:private test-card-regex (re-pattern card-name)) -(defn- produces-bytes? [{:keys [attachment-bytes-thunk]}] - (pos? (alength ^bytes (attachment-bytes-thunk)))) +(defn- produces-bytes? [{:keys [rendered-info]}] + (when rendered-info + (pos? (alength ^bytes (or (render/png-from-render-info rendered-info) + (byte-array 0)))))) (defn- email-body? [{message-type :type, ^String content :content}] (and (= "text/html; charset=utf-8" message-type) @@ -172,12 +175,13 @@ (is (= {:channel-id "#general" :message "Pulse: Pulse Name" :attachments - [{:title card-name - :attachment-bytes-thunk true - :title_link (str "https://metabase.com/testmb/question/" card-id) - :attachment-name "image.png" - :channel-id "FOO" - :fallback card-name}]} + [{:title card-name + :rendered-info {:attachments false + :content true} + :title_link (str "https://metabase.com/testmb/question/" card-id) + :attachment-name "image.png" + :channel-id "FOO" + :fallback card-name}]} (thunk->boolean pulse-results))))}})) (deftest basic-table-test @@ -205,20 +209,18 @@ :slack (fn [{:keys [card-id]} [pulse-results]] - ;; If we don't force the thunk, the rendering code will never execute and attached-results-text won't be - ;; called - (force-bytes-thunk pulse-results) (testing "\"more results in attachment\" text should not be present for Slack Pulses" (testing "Pulse results" (is (= {:channel-id "#general" :message "Pulse: Pulse Name" :attachments - [{:title card-name - :attachment-bytes-thunk true - :title_link (str "https://metabase.com/testmb/question/" card-id) - :attachment-name "image.png" - :channel-id "FOO" - :fallback card-name}]} + [{:title card-name + :rendered-info {:attachments false + :content true} + :title_link (str "https://metabase.com/testmb/question/" card-id) + :attachment-name "image.png" + :channel-id "FOO" + :fallback card-name}]} (thunk->boolean pulse-results)))) (testing "attached-results-text should be invoked exactly once" (is (= 1 @@ -423,7 +425,8 @@ (is (= {:channel-id "#general", :message "Alert: Test card", :attachments [{:title card-name - :attachment-bytes-thunk true + :rendered-info {:attachments false + :content true} :title_link (str "https://metabase.com/testmb/question/" card-id) :attachment-name "image.png" :channel-id "FOO" @@ -633,13 +636,15 @@ :message "Pulse: Pulse Name", :attachments [{:title card-name, - :attachment-bytes-thunk true, + :rendered-info {:attachments false + :content true} :title_link (str "https://metabase.com/testmb/question/" card-id-1), :attachment-name "image.png", :channel-id "FOO", :fallback card-name} {:title "Test card 2", - :attachment-bytes-thunk true + :rendered-info {:attachments false + :content true} :title_link (str "https://metabase.com/testmb/question/" card-id-2), :attachment-name "image.png", :channel-id "FOO", @@ -648,6 +653,47 @@ (testing "attachments" (is (true? (every? produces-bytes? (:attachments slack-data)))))))))) +(deftest create-and-upload-slack-attachments!-test + (let [slack-uploader (fn [storage] + (fn [_bytes attachment-name _channel-id] + (swap! storage conj attachment-name) + (str "http://uploaded/" attachment-name)))] + (testing "Uploads files" + (let [titles (atom []) + attachments [{:title "a" + :attachment-name "a.png" + :rendered-info {:attachments nil + :content [:div "hi"]} + :channel-id "FOO"} + {:title "b" + :attachment-name "b.png" + :rendered-info {:attachments nil + :content [:div "hi again"]} + :channel-id "FOO"}] + processed (pulse/create-and-upload-slack-attachments! attachments (slack-uploader titles))] + (is (= [{:title "a", :image_url "http://uploaded/a.png"} + {:title "b", :image_url "http://uploaded/b.png"}] + processed)) + (is (= @titles ["a.png" "b.png"])))) + (testing "Uses the raw text when present" + (let [titles (atom []) + attachments [{:title "a" + :attachment-name "a.png" + :rendered-info {:attachments nil + :content [:div "hi"]} + :channel-id "FOO"} + {:title "b" + :attachment-name "b.png" + :rendered-info {:attachments nil + :content [:div "hi again"] + :render/text "hi again"} + :channel-id "FOO"}] + processed (pulse/create-and-upload-slack-attachments! attachments (slack-uploader titles))] + (is (= [{:title "a", :image_url "http://uploaded/a.png"} + {:title "b", :text "hi again"}] + processed)) + (is (= @titles ["a.png"])))))) + (deftest multi-channel-test (testing "Test with a slack channel and an email" (mt/with-temp Card [{card-id :id} (checkins-query-card {:breakout [!day.date]})] @@ -663,14 +709,16 @@ email-data (m/find-first #(contains? % :subject) pulse-data)] (is (= {:channel-id "#general" :message "Pulse: Pulse Name" - :attachments [{:title card-name - :attachment-bytes-thunk true - :title_link (str "https://metabase.com/testmb/question/" card-id) - :attachment-name "image.png" - :channel-id "FOO" - :fallback card-name}]} + :attachments [{:title card-name + :title_link (str "https://metabase.com/testmb/question/" card-id) + :rendered-info {:attachments false + :content true} + :attachment-name "image.png" + :channel-id "FOO" + :fallback card-name}]} (thunk->boolean slack-data))) - (is (every? produces-bytes? (:attachments slack-data))) + (is (= [true] + (map (comp some? :content :rendered-info) (:attachments slack-data)))) (is (= {:subject "Pulse: Pulse Name", :recipients ["rasta@metabase.com"], :message-type :attachments} (select-keys email-data [:subject :recipients :message-type]))) (is (= 2 From f97e2deeff0f8fdc33f21bbe8efd61839e7121ce Mon Sep 17 00:00:00 2001 From: Ariya Hidayat Date: Fri, 9 Jul 2021 14:25:35 -0700 Subject: [PATCH 005/664] CI GitHub Actions: Cache JARs for Shadow CLJS (#16965) Some front-end tasks require Shadow CLJS to compile files into JavaScript, therefore let's cache the JARs which it needs. --- .github/workflows/frontend.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml index 4ebee9f2ac2f..8f499829c3ca 100644 --- a/.github/workflows/frontend.yml +++ b/.github/workflows/frontend.yml @@ -49,6 +49,11 @@ jobs: uses: actions/setup-node@v1 with: node-version: 14.x + - name: Get M2 cache + uses: actions/cache@v2 + with: + path: ~/.m2 + key: ${{ runner.os }}-cljs-${{ hashFiles('**/shadow-cljs.edn') }} - name: Get yarn cache uses: actions/cache@v2 with: @@ -85,6 +90,11 @@ jobs: uses: actions/setup-node@v1 with: node-version: 14.x + - name: Get M2 cache + uses: actions/cache@v2 + with: + path: ~/.m2 + key: ${{ runner.os }}-cljs-${{ hashFiles('**/shadow-cljs.edn') }} - name: Get yarn cache uses: actions/cache@v2 with: @@ -103,6 +113,11 @@ jobs: uses: actions/setup-node@v1 with: node-version: 14.x + - name: Get M2 cache + uses: actions/cache@v2 + with: + path: ~/.m2 + key: ${{ runner.os }}-cljs-${{ hashFiles('**/shadow-cljs.edn') }} - name: Get yarn cache uses: actions/cache@v2 with: From f6f4f0c794831245c258b05f634356c152c6af00 Mon Sep 17 00:00:00 2001 From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com> Date: Fri, 9 Jul 2021 23:29:48 +0200 Subject: [PATCH 006/664] [SQL filters coverage] Add initial set of e2e tests around SQL field filter type of "None" (#16928) * Add test for SQL field filter of the type "None" * Add repro for #13825 * Fix spelling Co-authored-by: Gustavo Saiani --- .../filters/sql-field-filter-none.cy.spec.js | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 frontend/test/metabase/scenarios/filters/sql-field-filter-none.cy.spec.js diff --git a/frontend/test/metabase/scenarios/filters/sql-field-filter-none.cy.spec.js b/frontend/test/metabase/scenarios/filters/sql-field-filter-none.cy.spec.js new file mode 100644 index 000000000000..40d451c02bd3 --- /dev/null +++ b/frontend/test/metabase/scenarios/filters/sql-field-filter-none.cy.spec.js @@ -0,0 +1,60 @@ +import { + restore, + mockSessionProperty, + openNativeEditor, + filterWidget, +} from "__support__/e2e/cypress"; + +import * as SQLFilter from "./helpers/e2e-sql-filter-helpers"; +import * as FieldFilter from "./helpers/e2e-field-filter-helpers"; + +describe("scenarios > filters > sql filters > field filter > None", () => { + beforeEach(() => { + restore(); + cy.intercept("POST", "api/dataset").as("dataset"); + + cy.signInAsAdmin(); + // Make sure feature flag is on regardless of the environment where this is running + mockSessionProperty("field-filter-operators-enabled?", true); + + openNativeEditor(); + SQLFilter.enterParameterizedQuery( + "SELECT * FROM products WHERE {{filter}}", + ); + + SQLFilter.openTypePickerFromDefaultFilterType(); + SQLFilter.chooseType("Field Filter"); + + FieldFilter.mapTo({ + table: "Products", + field: "Category", + }); + + FieldFilter.setWidgetType("None"); + + filterWidget().should("not.exist"); + }); + + it("should successfully run the query and show all results", () => { + SQLFilter.runQuery(); + + cy.get(".Visualization").within(() => { + cy.findByText("Rustic Paper Wallet"); + }); + + cy.findByText("Showing 200 rows"); + }); + + it.skip("should let you change the field filter type to something else and restore the filter widget (metabase#13825)", () => { + FieldFilter.setWidgetType("String contains"); + + FieldFilter.openEntryForm(); + FieldFilter.addWidgetStringFilter("zm"); + + SQLFilter.runQuery(); + + cy.get(".Visualization").within(() => { + cy.findByText("Rustic Paper Wallet"); + }); + }); +}); From 1d107a066456e9b19d7bbc477d06ca0089021cc5 Mon Sep 17 00:00:00 2001 From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com> Date: Sat, 10 Jul 2021 00:51:46 +0200 Subject: [PATCH 007/664] [SQL filters coverage] Add initial set of e2e tests around SQL field filter type of ID (#16926) * Add test for SQL field filter of the type ID * Fix spelling Co-authored-by: Gustavo Saiani --- .../filters/sql-field-filter-id.cy.spec.js | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 frontend/test/metabase/scenarios/filters/sql-field-filter-id.cy.spec.js diff --git a/frontend/test/metabase/scenarios/filters/sql-field-filter-id.cy.spec.js b/frontend/test/metabase/scenarios/filters/sql-field-filter-id.cy.spec.js new file mode 100644 index 000000000000..ba6c461a6773 --- /dev/null +++ b/frontend/test/metabase/scenarios/filters/sql-field-filter-id.cy.spec.js @@ -0,0 +1,56 @@ +import { + restore, + mockSessionProperty, + openNativeEditor, +} from "__support__/e2e/cypress"; + +import * as SQLFilter from "./helpers/e2e-sql-filter-helpers"; +import * as FieldFilter from "./helpers/e2e-field-filter-helpers"; + +describe("scenarios > filters > sql filters > field filter > ID", () => { + beforeEach(() => { + restore(); + cy.intercept("POST", "api/dataset").as("dataset"); + + cy.signInAsAdmin(); + // Make sure feature flag is on regardless of the environment where this is running + mockSessionProperty("field-filter-operators-enabled?", true); + + openNativeEditor(); + SQLFilter.enterParameterizedQuery( + "SELECT * FROM products WHERE {{filter}}", + ); + + SQLFilter.openTypePickerFromDefaultFilterType(); + SQLFilter.chooseType("Field Filter"); + + FieldFilter.mapTo({ + table: "Products", + field: "ID", + }); + + FieldFilter.setWidgetType("ID"); + }); + + it("should work when set initially as default value and then through the filter widget", () => { + SQLFilter.toggleRequired(); + + FieldFilter.openEntryForm({ isFilterRequired: true }); + FieldFilter.addDefaultStringFilter("2"); + + SQLFilter.runQuery(); + + cy.get(".Visualization").within(() => { + cy.findByText("Small Marble Shoes"); + }); + + FieldFilter.openEntryForm(); + FieldFilter.addWidgetStringFilter("1"); + + SQLFilter.runQuery(); + + cy.get(".Visualization").within(() => { + cy.findByText("Rustic Paper Wallet"); + }); + }); +}); From 3c71c5186dd4c9003b756f4db25498968283e0b3 Mon Sep 17 00:00:00 2001 From: Ariya Hidayat Date: Fri, 9 Jul 2021 21:19:25 -0700 Subject: [PATCH 008/664] Run unit tests with Jest with --silent (#16967) The console messages (mostly about React warning on deprecated methods) are useful when debugging a running application but they are atrocious, extremely noisy, and barely useful when running the unit tests. In the CI, those console messages drown the more useful PASS/FAIL status of each individual test. --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index b3cb6076f107..86034b925b24 100644 --- a/package.json +++ b/package.json @@ -167,9 +167,9 @@ "lint-docs-links": "yarn && ./bin/verify-doc-links", "lint-yaml": "yamllint **/*.{yaml,yml} --ignore=node_modules/**/*.{yaml,yml}", "test": "yarn test-unit && yarn test-timezones && yarn test-cypress", - "test-unit": "yarn build-quick:cljs && jest --maxWorkers=2 --config jest.unit.conf.json", + "test-unit": "yarn build-quick:cljs && jest --silent --maxWorkers=2 --config jest.unit.conf.json", "test-unit-watch": "yarn test-unit --watch", - "test-timezones-unit": "yarn build-quick:cljs && jest --maxWorkers=2 --config jest.tz.unit.conf.json", + "test-timezones-unit": "yarn build-quick:cljs && jest --silent --maxWorkers=2 --config jest.tz.unit.conf.json", "test-timezones": "yarn && ./frontend/test/__runner__/run_timezone_tests", "build:js": "yarn && webpack --bail", "build-watch:js": "yarn && webpack --watch", From 02fddd8a4f112d52f143a898483614c67be2c21c Mon Sep 17 00:00:00 2001 From: Anton Kulyk Date: Mon, 12 Jul 2021 15:45:51 +0300 Subject: [PATCH 009/664] Remove console logs (#16982) --- frontend/src/metabase/App.jsx | 1 - frontend/src/metabase/containers/Form.jsx | 2 -- frontend/src/metabase/lib/expressions/parser.js | 2 -- .../src/metabase/query_builder/components/AggregationWidget.jsx | 1 - .../query_builder/components/AlertListPopoverContent.jsx | 1 - .../query_builder/components/filters/pickers/TimePicker.jsx | 1 - .../src/metabase/visualizations/components/TableInteractive.jsx | 1 - 7 files changed, 9 deletions(-) diff --git a/frontend/src/metabase/App.jsx b/frontend/src/metabase/App.jsx index 6bacc9515ca9..dd176e986987 100644 --- a/frontend/src/metabase/App.jsx +++ b/frontend/src/metabase/App.jsx @@ -55,7 +55,6 @@ export default class App extends Component { } componentDidCatch(error, errorInfo) { - console.log("COMPONENT DID CATCH LOLE"); this.setState({ errorInfo }); } diff --git a/frontend/src/metabase/containers/Form.jsx b/frontend/src/metabase/containers/Form.jsx index 853e55717a74..c93174526f3f 100644 --- a/frontend/src/metabase/containers/Form.jsx +++ b/frontend/src/metabase/containers/Form.jsx @@ -255,7 +255,6 @@ export default class Form extends React.Component { _registerFormField = (field: FormFieldDefinition) => { if (!_.isEqual(this.state.inlineFields[field.name], field)) { - // console.log("_registerFormField", field.name); this.setState(prevState => assocIn(prevState, ["inlineFields", field.name], field), ); @@ -264,7 +263,6 @@ export default class Form extends React.Component { _unregisterFormField = (field: FormFieldDefinition) => { if (this.state.inlineFields[field.name]) { - // console.log("_unregisterFormField", field.name); // this.setState(prevState => // dissocIn(prevState, ["inlineFields", field.name]), // ); diff --git a/frontend/src/metabase/lib/expressions/parser.js b/frontend/src/metabase/lib/expressions/parser.js index a80ceccc14c4..074e7e16a392 100644 --- a/frontend/src/metabase/lib/expressions/parser.js +++ b/frontend/src/metabase/lib/expressions/parser.js @@ -391,12 +391,10 @@ export class ExpressionParser extends CstParser { } canTokenTypeBeInsertedInRecovery() { - // console.log("insert", this.tokenRecoveryEnabled); return this.tokenRecoveryEnabled; } canRecoverWithSingleTokenDeletion() { - // console.log("delete", this.tokenRecoveryEnabled); return this.tokenRecoveryEnabled; } } diff --git a/frontend/src/metabase/query_builder/components/AggregationWidget.jsx b/frontend/src/metabase/query_builder/components/AggregationWidget.jsx index 61216094c968..78bfd52583f0 100644 --- a/frontend/src/metabase/query_builder/components/AggregationWidget.jsx +++ b/frontend/src/metabase/query_builder/components/AggregationWidget.jsx @@ -48,7 +48,6 @@ export default class AggregationWidget extends React.Component { children, className, } = this.props; - console.log("aggregation", aggregation); const popover = this.state.isOpen && ( diff --git a/frontend/src/metabase/query_builder/components/AlertListPopoverContent.jsx b/frontend/src/metabase/query_builder/components/AlertListPopoverContent.jsx index 0260e1fbb527..26073b73b3a6 100644 --- a/frontend/src/metabase/query_builder/components/AlertListPopoverContent.jsx +++ b/frontend/src/metabase/query_builder/components/AlertListPopoverContent.jsx @@ -286,7 +286,6 @@ export class AlertScheduleText extends Component { return `${verbose ? "daily at " : "Daily, "} ${hour} ${amPm}`; } else if (scheduleType === "weekly") { - console.log(schedule); const hourOfDay = schedule.schedule_hour; const day = _.find( DAY_OF_WEEK_OPTIONS, diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/TimePicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/TimePicker.jsx index ac1140d75c3a..35e1f5fba642 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/TimePicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/TimePicker.jsx @@ -56,7 +56,6 @@ const MultiTimePicker = ({ filter, onFilterChange }) => ( ); const sortTimes = (a, b) => { - console.log(parseTime(a).isAfter(parseTime(b))); return parseTime(a).isAfter(parseTime(b)) ? [b, a] : [a, b]; }; diff --git a/frontend/src/metabase/visualizations/components/TableInteractive.jsx b/frontend/src/metabase/visualizations/components/TableInteractive.jsx index 6c9ed57e1fed..618d8dcf9bae 100644 --- a/frontend/src/metabase/visualizations/components/TableInteractive.jsx +++ b/frontend/src/metabase/visualizations/components/TableInteractive.jsx @@ -869,7 +869,6 @@ export default class TableInteractive extends Component { _benchmark() { const grid = ReactDOM.findDOMNode(this.grid); const height = grid.scrollHeight; - console.log("height", height); let top = 0; let start = Date.now(); // console.profile(); From bc2a3ac00ce01da88a4c4f568c8db09668ffcec5 Mon Sep 17 00:00:00 2001 From: Gustavo Saiani Date: Mon, 12 Jul 2021 11:01:52 -0300 Subject: [PATCH 010/664] =?UTF-8?q?Test=20CTA=20renders=20for=20Enterprise?= =?UTF-8?q?=20and=20Cloud=20in=20Settings=20=E2=80=BA=20Admin=20=E2=80=BA?= =?UTF-8?q?=20Updates=20(#16966)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../SettingsUpdatesForm.unit.spec.js | 58 +++++++++++++++++++ .../components}/DashCard.unit.spec.js | 0 jest.unit.conf.json | 3 +- 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 frontend/src/metabase/admin/settings/components/SettingsUpdatesForm.unit.spec.js rename frontend/{test/metabase/dashboard => src/metabase/dashboard/components}/DashCard.unit.spec.js (100%) diff --git a/frontend/src/metabase/admin/settings/components/SettingsUpdatesForm.unit.spec.js b/frontend/src/metabase/admin/settings/components/SettingsUpdatesForm.unit.spec.js new file mode 100644 index 000000000000..9afec230eb6c --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/SettingsUpdatesForm.unit.spec.js @@ -0,0 +1,58 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; + +import MetabaseSettings from "../../../../metabase/lib/settings"; +import SettingsUpdatesForm from "./SettingsUpdatesForm"; + +const elements = [ + { + key: "key", + }, +]; + +describe("SettingsUpdatesForm", () => { + it("shows custom message for Cloud installations", () => { + const isHostedSpy = jest.spyOn(MetabaseSettings, "isHosted"); + isHostedSpy.mockImplementation(() => true); + + render(); + screen.getByText(/Metabase Cloud keeps your instance up-to-date/); + + isHostedSpy.mockRestore(); + }); + + it("shows correct message when latest version is installed", () => { + const versionIsLatestSpy = jest.spyOn(MetabaseSettings, "versionIsLatest"); + versionIsLatestSpy.mockImplementation(() => true); + + render(); + screen.getByText(/which is the latest and greatest/); + + versionIsLatestSpy.mockRestore(); + }); + + it("shows correct message when no version checks have been run", () => { + render(); + screen.getByText("No successful checks yet."); + }); + + it("shows upgrade call-to-action if not in Enterprise plan", () => { + const versionIsLatestSpy = jest.spyOn(MetabaseSettings, "versionIsLatest"); + versionIsLatestSpy.mockImplementation(() => true); + + render(); + screen.getByText("Migrate to Metabase Cloud."); + + versionIsLatestSpy.mockRestore(); + }); + + it("does not show upgrade call-to-action if in Enterprise plan", () => { + const versionIsLatestSpy = jest.spyOn(MetabaseSettings, "versionIsLatest"); + versionIsLatestSpy.mockImplementation(() => false); + + render(); + expect(screen.queryByText("Migrate to Metabase Cloud.")).toBeNull(); + + versionIsLatestSpy.mockRestore(); + }); +}); diff --git a/frontend/test/metabase/dashboard/DashCard.unit.spec.js b/frontend/src/metabase/dashboard/components/DashCard.unit.spec.js similarity index 100% rename from frontend/test/metabase/dashboard/DashCard.unit.spec.js rename to frontend/src/metabase/dashboard/components/DashCard.unit.spec.js diff --git a/jest.unit.conf.json b/jest.unit.conf.json index b847388e3faf..f18ac6e3ceeb 100644 --- a/jest.unit.conf.json +++ b/jest.unit.conf.json @@ -6,8 +6,7 @@ }, "testPathIgnorePatterns": ["/frontend/test/.*/.*.tz.unit.spec.js"], "testMatch": [ - "/frontend/test/**/*.unit.spec.js?(x)", - "/enterprise/frontend/test/**/*.unit.spec.js?(x)" + "/**/*.unit.spec.js" ], "modulePaths": [ "/frontend/test", From 8df69c589a4a3ecac63557d096da949d993386f3 Mon Sep 17 00:00:00 2001 From: Ariya Hidayat Date: Mon, 12 Jul 2021 08:25:51 -0700 Subject: [PATCH 011/664] VS Code launch config to facilitate debugging (#16975) --- .vscode/launch.json | 22 ++++++++++++++++++++++ docs/developers-guide-vscode.md | 19 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 .vscode/launch.json diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 000000000000..679f283d3f2e --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,22 @@ +{ + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 + "version": "0.2.0", + "configurations": [ + { + "name": "Debug with Firefox", + "request": "launch", + "type": "firefox", + "url": "http://localhost:3000", + "webRoot": "${workspaceFolder}" + }, + { + "name": "Debug with Chrome", + "request": "launch", + "type": "pwa-chrome", + "url": "http://localhost:3000", + "webRoot": "${workspaceFolder}" + }, + ] +} \ No newline at end of file diff --git a/docs/developers-guide-vscode.md b/docs/developers-guide-vscode.md index 032a374f816b..97859c04d337 100644 --- a/docs/developers-guide-vscode.md +++ b/docs/developers-guide-vscode.md @@ -1,5 +1,24 @@ # Developing Metabase with Visual Studio Code +## Debugging + +First, install the following extensions: +* [Debugger for Firefox](https://marketplace.visualstudio.com/items?itemName=firefox-devtools.vscode-firefox-debug) +* [Debugger for Chrome](https://marketplace.visualstudio.com/items?itemName=msjsdiag.debugger-for-chrome) + +Before starting the debugging session, make sure that Metabase is built and running. Choose menu _View_, _Command Palette_, search for and choose _Tasks: Run Build Task_. Alternatively, use the corresponding shortcut `Ctrl+Shift+B`. The built-in terminal will appear to show the progress, wait a few moment until webpack indicates a complete (100%) bundling. + +To begin debugging Metabase, switch to the Debug view (shortcut: `Ctrl+Shift+D`) and then select one of the two launch configurations from the drop-down at the top: + +* Debug with Firefox, or +* Debug with Chrome + +After that, begin the debugging session by choosing menu _Run_, _Start Debugging_ (shortcut: `F5`). + +For more details, please refer to the complete VS Code documentation on [Debugging](https://code.visualstudio.com/docs/editor/debugging). + +## Docker-based Workflow + These instructions allow you to work on Metabase codebase on Windows, Linux, or macOS using [Visual Studio Code](https://code.visualstudio.com/), **without** manually installing the necessary dependencies. This is possible by leveraging Docker container and the Remote Containers extension from VS Code. For more details, please follow the complete VS Code guide on [Developing inside a Container](https://code.visualstudio.com/docs/remote/containers). The summary is as follows. From b96a477f0bcb4c7b0982f7d303dd46b985b58da5 Mon Sep 17 00:00:00 2001 From: Anton Kulyk Date: Mon, 12 Jul 2021 19:21:41 +0300 Subject: [PATCH 012/664] Fix console errors for Radio and SegmentedControl (#16950) --- .../components/PermissionsTabs.jsx | 2 +- .../src/metabase/components/Radio.info.js | 29 ++++--- frontend/src/metabase/components/Radio.jsx | 85 ++++++++++++------- .../src/metabase/components/Radio.styled.js | 31 ++++++- .../metabase/components/SegmentedControl.jsx | 48 ++++++----- .../components/SegmentedControl.styled.js | 16 +++- frontend/src/metabase/css/components/form.css | 22 ----- .../components/ChartSettings.jsx | 1 + .../components/ChartSettings.unit.spec.js | 26 +++--- 9 files changed, 160 insertions(+), 100 deletions(-) diff --git a/frontend/src/metabase/admin/permissions/components/PermissionsTabs.jsx b/frontend/src/metabase/admin/permissions/components/PermissionsTabs.jsx index 6b313b63ac45..40cb5accc8be 100644 --- a/frontend/src/metabase/admin/permissions/components/PermissionsTabs.jsx +++ b/frontend/src/metabase/admin/permissions/components/PermissionsTabs.jsx @@ -13,7 +13,7 @@ const PermissionsTabs = ({ tab, onChangeTab }) => ( { name: t`Data permissions`, value: `databases` }, { name: t`Collection permissions`, value: `collections` }, ]} - onChange={onChangeTab} + onOptionClick={onChangeTab} variant="underlined" py={2} /> diff --git a/frontend/src/metabase/components/Radio.info.js b/frontend/src/metabase/components/Radio.info.js index b5fb63b27127..afdcff56deda 100644 --- a/frontend/src/metabase/components/Radio.info.js +++ b/frontend/src/metabase/components/Radio.info.js @@ -1,4 +1,4 @@ -import React from "react"; +import React, { useState } from "react"; import Radio from "metabase/components/Radio"; export const component = Radio; @@ -8,15 +8,24 @@ export const description = ` A standard radio button group. `; -const PROPS = { - value: 0, - options: [{ name: "Gadget", value: 0 }, { name: "Gizmo", value: 1 }], -}; +const OPTIONS = [{ name: "Gadget", value: 0 }, { name: "Gizmo", value: 1 }]; + +function RadioDemo(props) { + const [value, setValue] = useState(0); + return ( + setValue(nextValue)} + /> + ); +} export const examples = { - default: , - underlined: , - "show buttons": , - vertical: , - bubble: , + default: , + underlined: , + "show buttons": , + vertical: , + bubble: , }; diff --git a/frontend/src/metabase/components/Radio.jsx b/frontend/src/metabase/components/Radio.jsx index 56041536f5ff..2a0dcff09bef 100644 --- a/frontend/src/metabase/components/Radio.jsx +++ b/frontend/src/metabase/components/Radio.jsx @@ -4,6 +4,8 @@ import _ from "underscore"; import Icon from "metabase/components/Icon"; import { + RadioInput, + RadioButton, BubbleList, BubbleItem, NormalList, @@ -25,8 +27,12 @@ const optionShape = PropTypes.shape({ const propTypes = { name: PropTypes.string, value: PropTypes.any, - options: PropTypes.arrayOf(optionShape).isRequired, + options: PropTypes.oneOfType([ + PropTypes.arrayOf(PropTypes.string), + PropTypes.arrayOf(optionShape), + ]).isRequired, onChange: PropTypes.func, + onOptionClick: PropTypes.func, optionNameFn: PropTypes.func, optionValueFn: PropTypes.func, optionKeyFn: PropTypes.func, @@ -51,9 +57,15 @@ const VARIANTS = { function Radio({ name: nameFromProps, - value, + value: currentValue, options, + + // onChange won't fire when you click an already checked item + // onOptionClick will fire in any case + // onOptionClick can be used for e.g. tab navigation like on the admin Permissions page) + onOptionClick, onChange, + optionNameFn = defaultNameGetter, optionValueFn = defaultValueGetter, optionKeyFn = defaultValueGetter, @@ -70,7 +82,7 @@ function Radio({ const [List, Item] = VARIANTS[variant] || VARIANTS.normal; - if (variant === "underlined" && value === undefined) { + if (variant === "underlined" && currentValue === undefined) { console.warn( "Radio can't underline selected option when no value is given.", ); @@ -79,35 +91,48 @@ function Radio({ return ( {options.map((option, index) => { - const selected = value === optionValueFn(option); + const value = optionValueFn(option); + const selected = currentValue === value; const last = index === options.length - 1; + const key = optionKeyFn(option); + const id = `${name}-${key}`; + const labelId = `${id}-label`; return ( - onChange(optionValueFn(option))} - aria-selected={selected} - > - {option.icon && } - - {showButtons && ( - +
    • + { + if (typeof onOptionClick === "function") { + onOptionClick(value); + } + }} + > + {option.icon && } + { + if (typeof onChange === "function") { + onChange(value); + } + }} + // Workaround for https://github.com/testing-library/dom-testing-library/issues/877 + aria-labelledby={labelId} + /> + {showButtons && } + {optionNameFn(option)} + +
      • ); })} diff --git a/frontend/src/metabase/components/Radio.styled.js b/frontend/src/metabase/components/Radio.styled.js index aaabf848a0e2..c7189d83d43f 100644 --- a/frontend/src/metabase/components/Radio.styled.js +++ b/frontend/src/metabase/components/Radio.styled.js @@ -4,17 +4,46 @@ import { space } from "styled-system"; import { color, lighten } from "metabase/lib/colors"; +export const RadioInput = styled.input.attrs({ type: "radio" })` + cursor: inherit; + position: absolute; + opacity: 0; + width: 0; + height: 0; + top: 0; + left: 0; + margin: 0; + padding: 0; + z-index: 1; +`; + +export const RadioButton = styled.div` + cursor: pointer; + display: inline-block; + flex: 0 0 auto; + position: relative; + margin-right: 0.5rem; + width: 12px; + height: 12px; + border: 2px solid white; + box-shadow: 0 0 0 2px ${color("shadow")}; + border-radius: 12px; + background-color: ${props => + props.checked ? color("brand") : "transparent"}; +`; + // BASE const BaseList = styled.ul` display: flex; flex-direction: ${props => (props.vertical ? "column" : "row")}; `; -const BaseItem = styled.li.attrs({ +const BaseItem = styled.label.attrs({ mr: props => (!props.vertical && !props.last ? props.xspace : null), mb: props => (props.vertical && !props.last ? props.yspace : null), })` ${space} + position: relative; display: flex; align-items: center; cursor: pointer; diff --git a/frontend/src/metabase/components/SegmentedControl.jsx b/frontend/src/metabase/components/SegmentedControl.jsx index 61ac026eb51c..1d42d47d7fd3 100644 --- a/frontend/src/metabase/components/SegmentedControl.jsx +++ b/frontend/src/metabase/components/SegmentedControl.jsx @@ -2,7 +2,11 @@ import React, { useMemo } from "react"; import PropTypes from "prop-types"; import _ from "underscore"; import Icon from "metabase/components/Icon"; -import { SegmentedList, SegmentedItem } from "./SegmentedControl.styled"; +import { + SegmentedList, + SegmentedItem, + SegmentedControlRadio, +} from "./SegmentedControl.styled"; const optionShape = PropTypes.shape({ name: PropTypes.node.isRequired, @@ -37,26 +41,30 @@ export function SegmentedControl({ const isSelected = option.value === value; const isFirst = index === 0; const isLast = index === options.length - 1; + const id = `${name}-${option.value}`; + const labelId = `${name}-${option.value}`; return ( - onChange(option.value)} - selectedColor={option.selectedColor || "brand"} - > - {option.icon && } - - {option.name} - +
    • + + {option.icon && } + onChange(option.value)} + // Workaround for https://github.com/testing-library/dom-testing-library/issues/877 + aria-labelledby={labelId} + /> + {option.name} + +
      • ); })} diff --git a/frontend/src/metabase/components/SegmentedControl.styled.js b/frontend/src/metabase/components/SegmentedControl.styled.js index 09e4651e1264..0728869dcb0a 100644 --- a/frontend/src/metabase/components/SegmentedControl.styled.js +++ b/frontend/src/metabase/components/SegmentedControl.styled.js @@ -7,7 +7,8 @@ export const SegmentedList = styled.ul` display: flex; `; -export const SegmentedItem = styled.li` +export const SegmentedItem = styled.label` + position: relative; display: flex; align-items: center; font-weight: bold; @@ -26,3 +27,16 @@ export const SegmentedItem = styled.li` color: ${props => (!props.isSelected ? color(props.selectedColor) : null)}; } `; + +export const SegmentedControlRadio = styled.input.attrs({ type: "radio" })` + cursor: inherit; + position: absolute; + opacity: 0; + width: 0; + height: 0; + top: 0; + left: 0; + margin: 0; + padding: 0; + z-index: 1; +`; diff --git a/frontend/src/metabase/css/components/form.css b/frontend/src/metabase/css/components/form.css index 48318462c7d8..53dc128379e6 100644 --- a/frontend/src/metabase/css/components/form.css +++ b/frontend/src/metabase/css/components/form.css @@ -104,28 +104,6 @@ transition: border 300ms ease-in-out; } -.Form-radio { - display: none; -} - -.Form-radio + label { - cursor: pointer; - display: inline-block; - flex: 0 0 auto; - position: relative; - margin-right: var(--margin-1); - width: 8px; - height: 8px; - border: 2px solid white; - box-shadow: 0 0 0 2px var(--color-shadow); - border-radius: 8px; -} - -.Form-radio:checked + label { - box-shadow: 0 0 0 2px var(--color-shadow); - background-color: var(--color-brand); -} - .Form-field .AdminSelect { border-color: var(--form-field-border-color); } diff --git a/frontend/src/metabase/visualizations/components/ChartSettings.jsx b/frontend/src/metabase/visualizations/components/ChartSettings.jsx index 5881ffc5fa50..0a193f84bf10 100644 --- a/frontend/src/metabase/visualizations/components/ChartSettings.jsx +++ b/frontend/src/metabase/visualizations/components/ChartSettings.jsx @@ -247,6 +247,7 @@ class ChartSettings extends Component { options={sectionNames} optionNameFn={v => v} optionValueFn={v => v} + optionKeyFn={v => v} variant="bubble" /> ); diff --git a/frontend/test/metabase/visualizations/components/ChartSettings.unit.spec.js b/frontend/test/metabase/visualizations/components/ChartSettings.unit.spec.js index c05aee5bd6a4..5a00af3d289c 100644 --- a/frontend/test/metabase/visualizations/components/ChartSettings.unit.spec.js +++ b/frontend/test/metabase/visualizations/components/ChartSettings.unit.spec.js @@ -20,10 +20,6 @@ function widget(widget = {}) { }; } -function expectTabSelected(node, value) { - expect(node.parentNode.getAttribute("aria-selected")).toBe(String(value)); -} - describe("ChartSettings", () => { it("should not crash if there are no widgets", () => { render(); @@ -38,17 +34,17 @@ describe("ChartSettings", () => { ); }); it("should default to the first section (if no section in DEFAULT_TAB_PRIORITY)", () => { - const { getByText } = render( + const { getByLabelText } = render( , ); - expectTabSelected(getByText("Foo"), true); - expectTabSelected(getByText("Bar"), false); + expect(getByLabelText("Foo")).toBeChecked(); + expect(getByLabelText("Bar")).not.toBeChecked(); }); it("should default to the DEFAULT_TAB_PRIORITY", () => { - const { getByText } = render( + const { getByLabelText } = render( { ]} />, ); - expectTabSelected(getByText("Foo"), false); - expectTabSelected(getByText("Display"), true); + expect(getByLabelText("Foo")).not.toBeChecked(); + expect(getByLabelText("Display")).toBeChecked(); }); it("should be able to switch sections", () => { - const { getByText } = render( + const { getByText, getByLabelText } = render( , ); - expectTabSelected(getByText("Foo"), true); - expectTabSelected(getByText("Bar"), false); + expect(getByLabelText("Foo")).toBeChecked(); + expect(getByLabelText("Bar")).not.toBeChecked(); fireEvent.click(getByText("Bar")); - expectTabSelected(getByText("Foo"), false); - expectTabSelected(getByText("Bar"), true); + expect(getByLabelText("Foo")).not.toBeChecked(); + expect(getByLabelText("Bar")).toBeChecked(); }); it("should show widget names", () => { From cde652ba880cb08d5ea42501145e98d376027905 Mon Sep 17 00:00:00 2001 From: Alexander Lesnenko Date: Mon, 12 Jul 2021 19:31:02 +0300 Subject: [PATCH 013/664] Update iframeResizer.js (#16952) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mário Valney --- package.json | 2 +- resources/frontend_client/app/iframeResizer.js | 8 ++++---- yarn.lock | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index 86034b925b24..1a91fb13c4d2 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "history": "3", "humanize-plus": "^1.8.1", "icepick": "2.3.0", - "iframe-resizer": "^3.5.11", + "iframe-resizer": "^4.3.2", "inflection": "^1.7.1", "isomorphic-fetch": "^2.2.1", "js-cookie": "^2.1.2", diff --git a/resources/frontend_client/app/iframeResizer.js b/resources/frontend_client/app/iframeResizer.js index b7738dfd6721..34adeb81925b 100644 --- a/resources/frontend_client/app/iframeResizer.js +++ b/resources/frontend_client/app/iframeResizer.js @@ -1,9 +1,9 @@ -/*! iFrame Resizer (iframeSizer.min.js ) - v3.5.11 - 2017-03-13 +/*! iFrame Resizer (iframeSizer.min.js ) - v4.3.2 - 2021-04-26 * Desc: Force cross domain iframes to size to content. * Requires: iframeResizer.contentWindow.min.js to be loaded into the target frame. - * Copyright: (c) 2017 David J. Bradshaw - dave@bradshaw.net + * Copyright: (c) 2021 David J. Bradshaw - dave@bradshaw.net * License: MIT */ -!function(a){"use strict";function b(a,b,c){"addEventListener"in window?a.addEventListener(b,c,!1):"attachEvent"in window&&a.attachEvent("on"+b,c)}function c(a,b,c){"removeEventListener"in window?a.removeEventListener(b,c,!1):"detachEvent"in window&&a.detachEvent("on"+b,c)}function d(){var a,b=["moz","webkit","o","ms"];for(a=0;ae&&(e=c,h(V,"Set "+d+" to min value")),e>b&&(e=b,h(V,"Set "+d+" to max value")),U[d]=""+e}function g(){function b(){function a(){var a=0,b=!1;for(h(V,"Checking connection is from allowed list of origins: "+d);aP[x]["max"+a])throw new Error("Value for min"+a+" can not be greater than max"+a)}b("Height"),b("Width"),a("maxHeight"),a("minHeight"),a("maxWidth"),a("minWidth")}function f(){var a=d&&d.id||S.id+F++;return null!==document.getElementById(a)&&(a+=F++),a}function g(a){return R=a,""===a&&(c.id=a=f(),G=(d||{}).log,R=a,h(a,"Added missing iframe ID: "+a+" ("+c.src+")")),a}function i(){switch(h(x,"IFrame scrolling "+(P[x].scrolling?"enabled":"disabled")+" for "+x),c.style.overflow=!1===P[x].scrolling?"hidden":"auto",P[x].scrolling){case!0:c.scrolling="yes";break;case!1:c.scrolling="no";break;default:c.scrolling=P[x].scrolling}}function k(){("number"==typeof P[x].bodyMargin||"0"===P[x].bodyMargin)&&(P[x].bodyMarginV1=P[x].bodyMargin,P[x].bodyMargin=""+P[x].bodyMargin+"px")}function l(){var a=P[x].firstRun,b=P[x].heightCalculationMethod in O;!a&&b&&r({iframe:c,height:0,width:0,type:"init"})}function m(){Function.prototype.bind&&(P[x].iframe.iFrameResizer={close:n.bind(null,P[x].iframe),resize:u.bind(null,"Window resize","resize",P[x].iframe),moveToAnchor:function(a){u("Move to anchor","moveToAnchor:"+a,P[x].iframe,x)},sendMessage:function(a){a=JSON.stringify(a),u("Send Message","message:"+a,P[x].iframe,x)}})}function o(d){function e(){u("iFrame.onload",d,c,a,!0),l()}b(c,"load",e),u("init",d,c,a,!0)}function p(a){if("object"!=typeof a)throw new TypeError("Options is not an object")}function q(a){for(var b in S)S.hasOwnProperty(b)&&(P[x][b]=a.hasOwnProperty(b)?a[b]:S[b])}function s(a){return""===a||"file://"===a?"*":a}function t(a){a=a||{},P[x]={firstRun:!0,iframe:c,remoteHost:c.src.split("/").slice(0,3).join("/")},p(a),q(a),P[x].targetOrigin=!0===P[x].checkOrigin?s(P[x].remoteHost):"*"}function w(){return x in P&&"iFrameResizer"in c}var x=g(c.id);w()?j(x,"Ignored iFrame, already setup."):(t(d),i(),e(),k(),o(v(x)),m())}function x(a,b){null===Q&&(Q=setTimeout(function(){Q=null,a()},b))}function y(){function a(){function a(a){function b(b){return"0px"===P[a].iframe.style[b]}function c(a){return null!==a.offsetParent}c(P[a].iframe)&&(b("height")||b("width"))&&u("Visibility change","resize",P[a].iframe,a)}for(var b in P)a(b)}function b(b){h("window","Mutation observed: "+b[0].target+" "+b[0].type),x(a,16)}function c(){var a=document.querySelector("body"),c={attributes:!0,attributeOldValue:!1,characterData:!0,characterDataOldValue:!1,childList:!0,subtree:!0},e=new d(b);e.observe(a,c)}var d=window.MutationObserver||window.WebKitMutationObserver;d&&c()}function z(a){function b(){B("Window "+a,"resize")}h("window","Trigger event: "+a),x(b,16)}function A(){function a(){B("Tab Visable","resize")}"hidden"!==document.visibilityState&&(h("document","Trigger event: Visiblity change"),x(a,16))}function B(a,b){function c(a){return"parent"===P[a].resizeFrom&&P[a].autoResize&&!P[a].firstRun}for(var d in P)c(d)&&u(a,b,document.getElementById(d),d)}function C(){b(window,"message",l),b(window,"resize",function(){z("resize")}),b(document,"visibilitychange",A),b(document,"-webkit-visibilitychange",A),b(window,"focusin",function(){z("focus")}),b(window,"focus",function(){z("focus")})}function D(){function b(a,b){function c(){if(!b.tagName)throw new TypeError("Object is not a valid DOM element");if("IFRAME"!==b.tagName.toUpperCase())throw new TypeError("Expected