From 639addfaaa08ef95af1fe78d417ea10d0eab1240 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Fri, 29 Oct 2021 11:58:12 +0300
Subject: [PATCH] Fix border width for the spinner (#18727)
---
.../metabase/components/LoadingSpinner.css | 44 -----------------
.../metabase/components/LoadingSpinner.jsx | 25 ----------
.../LoadingSpinner/LoadingSpinner.jsx | 19 ++++++++
.../LoadingSpinner/LoadingSpinner.styled.jsx | 47 +++++++++++++++++++
.../components/LoadingSpinner/index.js | 1 +
.../scenarios/dashboard/dashboard.cy.spec.js | 2 +-
.../scenarios/dashboard/x-rays.cy.spec.js | 2 +-
.../onboarding/home/activity-page.cy.spec.js | 2 +-
.../search/recently-viewed.cy.spec.js | 2 +-
.../search/search-typehead.cy.spec.js | 2 +-
.../scenarios/question/filter.cy.spec.js | 2 +-
.../scenarios/question/nulls.cy.spec.js | 4 +-
...es-show-row-details-on-pk-click.cy.spec.js | 2 +-
13 files changed, 76 insertions(+), 78 deletions(-)
delete mode 100644 frontend/src/metabase/components/LoadingSpinner.css
delete mode 100644 frontend/src/metabase/components/LoadingSpinner.jsx
create mode 100644 frontend/src/metabase/components/LoadingSpinner/LoadingSpinner.jsx
create mode 100644 frontend/src/metabase/components/LoadingSpinner/LoadingSpinner.styled.jsx
create mode 100644 frontend/src/metabase/components/LoadingSpinner/index.js
diff --git a/frontend/src/metabase/components/LoadingSpinner.css b/frontend/src/metabase/components/LoadingSpinner.css
deleted file mode 100644
index 4294d35c247..00000000000
--- a/frontend/src/metabase/components/LoadingSpinner.css
+++ /dev/null
@@ -1,44 +0,0 @@
-.LoadingSpinner {
- display: inline-block;
- box-sizing: border-box;
- width: 32px;
- height: 32px;
- border: 4px solid transparent;
- border-top-color: currentColor;
- border-radius: 99px;
-
- animation: LoadingSpinner-transition 1.3s infinite
- cubic-bezier(0.785, 0.135, 0.15, 0.86);
-}
-
-@media (prefers-reduced-motion) {
- .LoadingSpinner {
- animation: none;
- }
-}
-
-.LoadingSpinner:after {
- content: "";
-
- display: inherit;
- box-sizing: inherit;
- width: inherit;
- height: inherit;
- border: inherit;
- border-color: currentColor;
- border-radius: inherit;
-
- opacity: 0.25;
- position: relative;
- top: -4px;
- left: -4px;
-}
-
-@keyframes LoadingSpinner-transition {
- from {
- transform: rotate(0deg);
- }
- to {
- transform: rotate(360deg);
- }
-}
diff --git a/frontend/src/metabase/components/LoadingSpinner.jsx b/frontend/src/metabase/components/LoadingSpinner.jsx
deleted file mode 100644
index dee77f59274..00000000000
--- a/frontend/src/metabase/components/LoadingSpinner.jsx
+++ /dev/null
@@ -1,25 +0,0 @@
-/* eslint-disable react/prop-types */
-import React, { Component } from "react";
-
-import "./LoadingSpinner.css";
-
-export default class LoadingSpinner extends Component {
- static defaultProps = {
- size: 32,
- borderWidth: 4,
- fill: "currentcolor",
- spinnerClassName: "LoadingSpinner",
- };
-
- render() {
- const { size, borderWidth, className, spinnerClassName } = this.props;
- return (
- <div className={className}>
- <div
- className={spinnerClassName}
- style={{ width: size, height: size, borderWidth }}
- />
- </div>
- );
- }
-}
diff --git a/frontend/src/metabase/components/LoadingSpinner/LoadingSpinner.jsx b/frontend/src/metabase/components/LoadingSpinner/LoadingSpinner.jsx
new file mode 100644
index 00000000000..c3513edb3c0
--- /dev/null
+++ b/frontend/src/metabase/components/LoadingSpinner/LoadingSpinner.jsx
@@ -0,0 +1,19 @@
+import React from "react";
+import PropTypes from "prop-types";
+import { SpinnerIcon, SpinnerRoot } from "./LoadingSpinner.styled";
+
+const propTypes = {
+ className: PropTypes.string,
+ size: PropTypes.number,
+ borderWidth: PropTypes.number,
+};
+
+const LoadingSpinner = ({ className, size = 32, borderWidth = 4 }) => (
+ <SpinnerRoot className={className} data-testid="loading-spinner">
+ <SpinnerIcon iconSize={size} borderWidth={borderWidth} />
+ </SpinnerRoot>
+);
+
+LoadingSpinner.propTypes = propTypes;
+
+export default LoadingSpinner;
diff --git a/frontend/src/metabase/components/LoadingSpinner/LoadingSpinner.styled.jsx b/frontend/src/metabase/components/LoadingSpinner/LoadingSpinner.styled.jsx
new file mode 100644
index 00000000000..fea9264b968
--- /dev/null
+++ b/frontend/src/metabase/components/LoadingSpinner/LoadingSpinner.styled.jsx
@@ -0,0 +1,47 @@
+import styled, { keyframes } from "styled-components";
+
+const spinnerAnimation = keyframes`
+ from {
+ transform: rotate(0deg);
+ }
+ to {
+ transform: rotate(360deg);
+ }
+`;
+
+export const SpinnerRoot = styled.div`
+ font-size: 0;
+`;
+
+export const SpinnerIcon = styled.div`
+ display: inline-block;
+ box-sizing: border-box;
+ width: ${props => `${props.iconSize}px`};
+ height: ${props => `${props.iconSize}px`};
+ border: ${props => `${props.borderWidth}px`} solid transparent;
+ border-top-color: currentColor;
+ border-radius: ${props => `${props.iconSize / 2}px`};
+
+ animation: ${spinnerAnimation} 1.3s infinite
+ cubic-bezier(0.785, 0.135, 0.15, 0.86);
+
+ &::after {
+ content: "";
+
+ display: inherit;
+ box-sizing: inherit;
+ width: inherit;
+ height: inherit;
+ border: ${props => `${props.borderWidth}px`} solid currentColor;
+ border-radius: ${props => `${props.iconSize / 2}px`};
+
+ opacity: 0.25;
+ position: relative;
+ top: ${props => `-${props.borderWidth}px`};
+ left: ${props => `-${props.borderWidth}px`};
+ }
+
+ @media (prefers-reduced-motion) {
+ animation: none;
+ }
+`;
diff --git a/frontend/src/metabase/components/LoadingSpinner/index.js b/frontend/src/metabase/components/LoadingSpinner/index.js
new file mode 100644
index 00000000000..443fa62feb8
--- /dev/null
+++ b/frontend/src/metabase/components/LoadingSpinner/index.js
@@ -0,0 +1 @@
+export { default } from "./LoadingSpinner";
diff --git a/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js
index 9e491d41f18..d4a0262c52d 100644
--- a/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js
+++ b/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js
@@ -425,7 +425,7 @@ describe("scenarios > dashboard", () => {
cy.findByText("Orders, Count").click();
});
- cy.get(".LoadingSpinner").should("not.exist");
+ cy.findByTestId("loading-spinner").should("not.exist");
cy.findAllByText("18,760").should("have.length", 2);
});
});
diff --git a/frontend/test/metabase/scenarios/dashboard/x-rays.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/x-rays.cy.spec.js
index bb4f9676e4d..3097946c348 100644
--- a/frontend/test/metabase/scenarios/dashboard/x-rays.cy.spec.js
+++ b/frontend/test/metabase/scenarios/dashboard/x-rays.cy.spec.js
@@ -141,7 +141,7 @@ describe("scenarios > x-rays", () => {
cy.contains("A look at your Orders table").click();
// There are a lot of spinners in this dashboard. Give them some time to disappear.
- cy.get(".LoadingSpinner", { timeout: 10000 }).should("not.exist");
+ cy.findByTestId("loading-spinner", { timeout: 10000 }).should("not.exist");
cy.button("Save this").click();
diff --git a/frontend/test/metabase/scenarios/onboarding/home/activity-page.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/home/activity-page.cy.spec.js
index d7a25ce59ad..d3474cac63c 100644
--- a/frontend/test/metabase/scenarios/onboarding/home/activity-page.cy.spec.js
+++ b/frontend/test/metabase/scenarios/onboarding/home/activity-page.cy.spec.js
@@ -68,7 +68,7 @@ describe("metabase > scenarios > home > activity-page", () => {
.click();
sidebar().within(() => {
- cy.get(".LoadingSpinner").should("not.exist");
+ cy.findByTestId("loading-spinner").should("not.exist");
cy.findByText("Orders").click();
});
diff --git a/frontend/test/metabase/scenarios/onboarding/search/recently-viewed.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/search/recently-viewed.cy.spec.js
index b54283a48af..742a3fb84b8 100644
--- a/frontend/test/metabase/scenarios/onboarding/search/recently-viewed.cy.spec.js
+++ b/frontend/test/metabase/scenarios/onboarding/search/recently-viewed.cy.spec.js
@@ -23,7 +23,7 @@ describe(`search > recently viewed`, () => {
cy.visit("/");
cy.findByPlaceholderText("Search…").click();
- cy.get(".LoadingSpinner").should("not.exist");
+ cy.findByTestId("loading-spinner").should("not.exist");
assertRecentlyViewedItem(0, "Orders", "Question", "/question/1-orders");
assertRecentlyViewedItem(
diff --git a/frontend/test/metabase/scenarios/onboarding/search/search-typehead.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/search/search-typehead.cy.spec.js
index b6e468a42fc..790db081ffd 100644
--- a/frontend/test/metabase/scenarios/onboarding/search/search-typehead.cy.spec.js
+++ b/frontend/test/metabase/scenarios/onboarding/search/search-typehead.cy.spec.js
@@ -15,7 +15,7 @@ import { USERS } from "__support__/e2e/cypress_data";
user === "admin" ? Object.entries(USERS).length : 1;
cy.findByPlaceholderText("Search…").type("pers");
- cy.get(".LoadingSpinner").should("not.exist");
+ cy.findByTestId("loading-spinner").should("not.exist");
cy.findAllByText(/personal collection$/i).should(
"have.length",
personalCollectionsLength,
diff --git a/frontend/test/metabase/scenarios/question/filter.cy.spec.js b/frontend/test/metabase/scenarios/question/filter.cy.spec.js
index 958486c4d38..07c7be4efa7 100644
--- a/frontend/test/metabase/scenarios/question/filter.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/filter.cy.spec.js
@@ -291,7 +291,7 @@ describe("scenarios > question > filter", () => {
cy.findByText("Add filter").click();
cy.log("Reported failing on v0.36.4 and v0.36.5.1");
- cy.get(".LoadingSpinner").should("not.exist");
+ cy.findByTestId("loading-spinner").should("not.exist");
cy.findAllByText("148.23"); // one of the subtotals for this product
cy.findAllByText("Fantastic Wool Shirt").should("not.exist");
});
diff --git a/frontend/test/metabase/scenarios/question/nulls.cy.spec.js b/frontend/test/metabase/scenarios/question/nulls.cy.spec.js
index 5990815092a..4af170b0d09 100644
--- a/frontend/test/metabase/scenarios/question/nulls.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/nulls.cy.spec.js
@@ -99,7 +99,7 @@ describe("scenarios > question > null", () => {
cy.log("Reported failing in v0.37.0.2");
cy.get(".DashCard").within(() => {
- cy.get(".LoadingSpinner").should("not.exist");
+ cy.findByTestId("loading-spinner").should("not.exist");
cy.findByText("13626");
// [quarantine]: flaking in CircleCI, passing locally
// TODO: figure out the cause of the failed test in CI after #13721 is merged
@@ -150,7 +150,7 @@ describe("scenarios > question > null", () => {
cy.visit(`/dashboard/${DASHBOARD_ID}`);
cy.log("P0 regression in v0.37.1!");
- cy.get(".LoadingSpinner").should("not.exist");
+ cy.findByTestId("loading-spinner").should("not.exist");
cy.findByText("13801_Q1");
cy.get(".ScalarValue").contains("0");
cy.findByText("13801_Q2");
diff --git a/frontend/test/metabase/scenarios/question/reproductions/13263-postgres-show-row-details-on-pk-click.cy.spec.js b/frontend/test/metabase/scenarios/question/reproductions/13263-postgres-show-row-details-on-pk-click.cy.spec.js
index 261d1940ddc..77cf3dd1e8b 100644
--- a/frontend/test/metabase/scenarios/question/reproductions/13263-postgres-show-row-details-on-pk-click.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/reproductions/13263-postgres-show-row-details-on-pk-click.cy.spec.js
@@ -22,7 +22,7 @@ describe("postgres > user > query", () => {
// Wait until "doing science" spinner disappears (DOM is ready for assertions)
// TODO: if this proves to be reliable, extract it as a helper function for waiting on DOM to render
- cy.get(".LoadingSpinner").should("not.exist");
+ cy.findByTestId("loading-spinner").should("not.exist");
// Assertions
cy.log("Fails in v0.36.6");
--
GitLab