From e082aef1daab52cd5f1f1e1ace4f78126a763a02 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 20 Sep 2023 18:09:14 -0500 Subject: [PATCH 01/20] CSS based loading spinner #1531 --- packages/components/scss/BaseStyleSheet.scss | 26 +++++++++--- packages/components/src/LoadingOverlay.scss | 8 +++- packages/components/src/LoadingOverlay.tsx | 8 +++- packages/components/src/LoadingSpinner.scss | 42 ++++++++++++++++++-- packages/components/src/LoadingSpinner.tsx | 18 +++++---- packages/utils/src/DOMUtils.ts | 27 ++++++++++++- 6 files changed, 108 insertions(+), 21 deletions(-) diff --git a/packages/components/scss/BaseStyleSheet.scss b/packages/components/scss/BaseStyleSheet.scss index 06bb64d500..10acf0ab2c 100644 --- a/packages/components/scss/BaseStyleSheet.scss +++ b/packages/components/scss/BaseStyleSheet.scss @@ -174,8 +174,10 @@ span.btn-disabled-wrapper { .btn-spinner { padding: $btn-padding-y 1rem; - .fa-layers { + .fa-layers, + .loading-spinner { margin-right: 0.5rem; + vertical-align: -0.125em; } } @@ -255,7 +257,9 @@ span.btn-disabled-wrapper { border-radius: 2px; margin-bottom: 2px; filter: saturate(0%); - transition: filter 0.15s ease-in-out, box-shadow 0.15s ease-in-out; + transition: + filter 0.15s ease-in-out, + box-shadow 0.15s ease-in-out; pointer-events: none; } @@ -712,7 +716,8 @@ input[type='number']::-webkit-inner-spin-button { .monaco-checkbox:focus { border: none !important; - box-shadow: $input-btn-focus-box-shadow, + box-shadow: + $input-btn-focus-box-shadow, 0 0 0 1px $input-focus-border-color; //can't use regular border due to position of checkbox } } @@ -746,7 +751,9 @@ input[type='number']::-webkit-inner-spin-button { linear-gradient(0deg, $input-bg, $input-bg); background-size: $custom-select-bg-size, cover; background-repeat: no-repeat; - background-position: bottom 50% right $custom-select-padding-x, center; + background-position: + bottom 50% right $custom-select-padding-x, + center; //make the dotted duplicate focus line on firefox go away &:-moz-focusring { color: rgba(0, 0, 0, 0%); @@ -835,11 +842,18 @@ input[type='number']::-webkit-inner-spin-button { /// used by marching ants animation @keyframes march { from { - background-position: 0 top, 0 bottom, left 0, right 0; + background-position: + 0 top, + 0 bottom, + left 0, + right 0; } to { - background-position: $ant-size top, $ant-size bottom, left $ant-size, + background-position: + $ant-size top, + $ant-size bottom, + left $ant-size, right $ant-size; } } diff --git a/packages/components/src/LoadingOverlay.scss b/packages/components/src/LoadingOverlay.scss index af9b6c3089..6d4d09e1ed 100644 --- a/packages/components/src/LoadingOverlay.scss +++ b/packages/components/src/LoadingOverlay.scss @@ -2,6 +2,7 @@ $iris-panel-message-font-size: 1.2rem; $iris-panel-icon-font-size: 64px; +$iris-panel-icon-height: 96px; $iris-panel-scrim-bg: rgba(black, 0.1); .iris-panel-message-overlay { @@ -15,9 +16,12 @@ $iris-panel-scrim-bg: rgba(black, 0.1); .message-content { font-size: $iris-panel-message-font-size; .message-icon { - font-size: $iris-panel-icon-font-size; + display: flex; + height: $iris-panel-icon-height; + align-items: center; + justify-content: center; .svg-inline--fa { - font-size: inherit; + font-size: $iris-panel-icon-font-size; } } } diff --git a/packages/components/src/LoadingOverlay.tsx b/packages/components/src/LoadingOverlay.tsx index 140f7c94b1..e193f00d82 100644 --- a/packages/components/src/LoadingOverlay.tsx +++ b/packages/components/src/LoadingOverlay.tsx @@ -27,6 +27,7 @@ function LoadingOverlay({ dataTestId != null ? `${dataTestId}-message` : undefined; const spinnerTestId = dataTestId != null ? `${dataTestId}-spinner` : undefined; + return (
- {isLoading && } + {isLoading && ( + + )} {!isLoading && errorMessage != null && ( )} diff --git a/packages/components/src/LoadingSpinner.scss b/packages/components/src/LoadingSpinner.scss index c401903ca2..f0e5da5938 100644 --- a/packages/components/src/LoadingSpinner.scss +++ b/packages/components/src/LoadingSpinner.scss @@ -1,6 +1,42 @@ +.loading-spinner { + --primary-color: var(--dh-accent-color, #4c7dee); + /* stylelint-disable-next-line alpha-value-notation */ + --secondary-color: rgba(240, 240, 240, 0.5); + --border-width: 1px; + --size: 14px; + + box-sizing: border-box; + display: inline-block; + margin: 0 auto; + width: var(--size); + height: var(--size); +} + .loading-spinner-large { - font-size: 64px; - .svg-inline--fa { - font-size: inherit; + --border-width: 4px; + --size: 56px; +} + +// Spinning circle with colored border and transparent center. Half of the +// circle is the primary color. The other half is the secondary color. +.loading-spinner::after { + animation: loading-spinner-rotate 2s linear infinite; + border: var(--border-width) solid; + border-color: var(--primary-color) var(--primary-color) var(--secondary-color) + var(--secondary-color); + border-radius: 50%; + box-sizing: border-box; + content: ''; + display: inline-block; + width: var(--size); + height: var(--size); +} + +@keyframes loading-spinner-rotate { + 0% { + transform: rotate(0deg); + } + 100% { + transform: rotate(359deg); } } diff --git a/packages/components/src/LoadingSpinner.tsx b/packages/components/src/LoadingSpinner.tsx index 56e4620218..be21bc1d5d 100644 --- a/packages/components/src/LoadingSpinner.tsx +++ b/packages/components/src/LoadingSpinner.tsx @@ -1,7 +1,6 @@ -import React from 'react'; +import { useLayoutEffect } from 'react'; import classNames from 'classnames'; -import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { vsCircleLarge, vsLoading } from '@deephaven/icons'; +import { DOMUtils } from '@deephaven/utils'; import './LoadingSpinner.scss'; type LoadingSpinnerProps = { @@ -13,14 +12,17 @@ function LoadingSpinner({ className = '', 'data-testid': dataTestId, }: LoadingSpinnerProps): JSX.Element { + useLayoutEffect(() => { + // Ensure all of our loading spinner animations are synchronized based + // on same start time. + DOMUtils.syncAnimationStartTime('loading-spinner-rotate', 0); + }, []); + return (
- - -
+ /> ); } diff --git a/packages/utils/src/DOMUtils.ts b/packages/utils/src/DOMUtils.ts index a314c91364..56dc0f5b17 100644 --- a/packages/utils/src/DOMUtils.ts +++ b/packages/utils/src/DOMUtils.ts @@ -28,4 +28,29 @@ export function identityExtractHTMLElement( return maybeHTMLElement instanceof HTMLElement ? maybeHTMLElement : null; } -export default { getClosestByClassName, identityExtractHTMLElement }; +/** + * Synchronize CSS animations with a given name to the same start time. + * @param animationName + * @param startTime + */ +export function syncAnimationStartTime( + animationName: string, + startTime: number +): void { + const animations = document + .getAnimations() + .filter( + a => a instanceof CSSAnimation && a.animationName === animationName + ); + + animations.forEach(a => { + // eslint-disable-next-line no-param-reassign + a.startTime = startTime; + }); +} + +export default { + getClosestByClassName, + identityExtractHTMLElement, + syncAnimationStartTime, +}; From 0c8ac73997ee117a5ba66ede1de3743a2b7e3eec Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 09:02:15 -0500 Subject: [PATCH 02/20] Semantic spinner colors #1531 --- packages/components/src/LoadingSpinner.scss | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/components/src/LoadingSpinner.scss b/packages/components/src/LoadingSpinner.scss index f0e5da5938..7a7d03b074 100644 --- a/packages/components/src/LoadingSpinner.scss +++ b/packages/components/src/LoadingSpinner.scss @@ -1,7 +1,13 @@ .loading-spinner { - --primary-color: var(--dh-accent-color, #4c7dee); - /* stylelint-disable-next-line alpha-value-notation */ - --secondary-color: rgba(240, 240, 240, 0.5); + --primary-color: var( + --dh-loading-spinner-primary-color, + --dh-accent-color, + #4c7dee + ); + --secondary-color: var( + --dh-loading-spinner-secondary-color, + rgba(240, 240, 240, 0.5) + ); --border-width: 1px; --size: 14px; From b6189d15e827fc45b0a340618828001c6387fccb Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 09:14:31 -0500 Subject: [PATCH 03/20] Fixed invalid css variable #1531 --- packages/components/src/LoadingSpinner.scss | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/components/src/LoadingSpinner.scss b/packages/components/src/LoadingSpinner.scss index 7a7d03b074..3fd9089d9a 100644 --- a/packages/components/src/LoadingSpinner.scss +++ b/packages/components/src/LoadingSpinner.scss @@ -1,8 +1,7 @@ .loading-spinner { --primary-color: var( --dh-loading-spinner-primary-color, - --dh-accent-color, - #4c7dee + var(--dh-accent-color, #4c7dee) ); --secondary-color: var( --dh-loading-spinner-secondary-color, From 299b38f308b08668c07e841466c118227a71d789 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 09:27:33 -0500 Subject: [PATCH 04/20] Disabled stylelint rule #1531 --- packages/components/src/LoadingSpinner.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/LoadingSpinner.scss b/packages/components/src/LoadingSpinner.scss index 3fd9089d9a..6c6931e194 100644 --- a/packages/components/src/LoadingSpinner.scss +++ b/packages/components/src/LoadingSpinner.scss @@ -1,3 +1,4 @@ +/* stylelint-disable alpha-value-notation */ .loading-spinner { --primary-color: var( --dh-loading-spinner-primary-color, From 814d6583e1d210e9af8b83f1455890abcacdf3fc Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 10:06:29 -0500 Subject: [PATCH 05/20] syncAnimationStartTime tests #1531 --- jest.setup.ts | 10 ++++++ packages/utils/src/DOMUtils.test.ts | 54 +++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/jest.setup.ts b/jest.setup.ts index cd53ccf1fe..4a831fcb39 100644 --- a/jest.setup.ts +++ b/jest.setup.ts @@ -33,6 +33,12 @@ Object.defineProperty(window, 'matchMedia', { })), }); +Object.defineProperty(window, 'CSSAnimation', { + value: function () { + return class CSSAnimation {}; + }, +}); + Object.defineProperty(window, 'performance', { value: performance, writable: true, @@ -57,3 +63,7 @@ Object.defineProperty(document, 'fonts', { ready: Promise.resolve(), }, }); + +Object.defineProperty(document, 'getAnimations', { + value: jest.fn().mockName('getAnimations'), +}); diff --git a/packages/utils/src/DOMUtils.test.ts b/packages/utils/src/DOMUtils.test.ts index d793546a7b..6ed65c4726 100644 --- a/packages/utils/src/DOMUtils.test.ts +++ b/packages/utils/src/DOMUtils.test.ts @@ -1,7 +1,16 @@ -import { getClosestByClassName, identityExtractHTMLElement } from './DOMUtils'; +import { + getClosestByClassName, + identityExtractHTMLElement, + syncAnimationStartTime, +} from './DOMUtils'; import TestUtils from './TestUtils'; -const { createMockProxy } = TestUtils; +const { asMock, createMockProxy } = TestUtils; + +beforeEach(() => { + jest.clearAllMocks(); + expect.hasAssertions(); +}); describe('getClosestByClassName', () => { let originalBodyHTML = document.body.innerHTML; @@ -65,3 +74,44 @@ describe('identityExtractHTMLElement', () => { expect(identityExtractHTMLElement(element)).toBe(element); }); }); + +describe('syncAnimationStartTime', () => { + // Mock a CSSAnimation that satisfies instanceof operator + const mockCSSAnimation = (name: string): CSSAnimation => { + const animation = Object.create(CSSAnimation.prototype); + animation.animationName = name; + return animation; + }; + + const cssAnimationA1 = mockCSSAnimation('animationA'); + const cssAnimationA2 = mockCSSAnimation('animationA'); + const cssAnimationB = mockCSSAnimation('animationB'); + const notCSSAnimation = { animationName: '' } as unknown as Animation; + + beforeEach(() => { + cssAnimationA1.startTime = null; + cssAnimationA2.startTime = null; + cssAnimationB.startTime = null; + notCSSAnimation.startTime = null; + }); + + it('should set startTime of all CSSAnimations with given name to given value', () => { + expect(cssAnimationA1).toBeInstanceOf(CSSAnimation); + + asMock(document.getAnimations).mockReturnValue([ + cssAnimationA1, + cssAnimationA2, + cssAnimationB, + notCSSAnimation, + ]); + + const startTime = 123; + + syncAnimationStartTime('animationA', startTime); + + expect(cssAnimationA1.startTime).toBe(startTime); + expect(cssAnimationA2.startTime).toBe(startTime); + expect(cssAnimationB.startTime).toBeNull(); + expect(notCSSAnimation.startTime).toBeNull(); + }); +}); From 7596430449d21868486374c0259b205f68d7a91d Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 10:21:15 -0500 Subject: [PATCH 06/20] Adjusted loading overlay styles #1531 --- packages/components/src/LoadingOverlay.scss | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/components/src/LoadingOverlay.scss b/packages/components/src/LoadingOverlay.scss index 6d4d09e1ed..55cc1cbf97 100644 --- a/packages/components/src/LoadingOverlay.scss +++ b/packages/components/src/LoadingOverlay.scss @@ -2,7 +2,6 @@ $iris-panel-message-font-size: 1.2rem; $iris-panel-icon-font-size: 64px; -$iris-panel-icon-height: 96px; $iris-panel-scrim-bg: rgba(black, 0.1); .iris-panel-message-overlay { @@ -14,14 +13,16 @@ $iris-panel-scrim-bg: rgba(black, 0.1); overflow: hidden; .message-content { + display: flex; + flex-direction: column; font-size: $iris-panel-message-font-size; + gap: 20px; .message-icon { - display: flex; - height: $iris-panel-icon-height; - align-items: center; - justify-content: center; + font-size: $iris-panel-icon-font-size; + height: $iris-panel-icon-font-size; + line-height: $iris-panel-icon-font-size; .svg-inline--fa { - font-size: $iris-panel-icon-font-size; + font-size: inherit; } } } From 611d28d0a58fc4cd1a0094bc411d4b9b5ab15063 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 10:36:10 -0500 Subject: [PATCH 07/20] Added style comment #1531 --- packages/components/scss/BaseStyleSheet.scss | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/components/scss/BaseStyleSheet.scss b/packages/components/scss/BaseStyleSheet.scss index 10acf0ab2c..a2f67e745b 100644 --- a/packages/components/scss/BaseStyleSheet.scss +++ b/packages/components/scss/BaseStyleSheet.scss @@ -177,6 +177,8 @@ span.btn-disabled-wrapper { .fa-layers, .loading-spinner { margin-right: 0.5rem; + // .fa-layers already uses this `vertical-align` to align spinner with + // button text. Setting explicilty so that .loading-spinner aligns the same vertical-align: -0.125em; } } From e37aacf7ec8a8622fb160ba5ebe4167d4000f608 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 10:52:30 -0500 Subject: [PATCH 08/20] Fixed failing unit tests #1531 --- jest.setup.ts | 3 +- .../ContextActions.test.tsx.snap | 78 +------------------ packages/utils/src/DOMUtils.test.ts | 17 ++-- 3 files changed, 16 insertions(+), 82 deletions(-) diff --git a/jest.setup.ts b/jest.setup.ts index 4a831fcb39..dbfe815f73 100644 --- a/jest.setup.ts +++ b/jest.setup.ts @@ -65,5 +65,6 @@ Object.defineProperty(document, 'fonts', { }); Object.defineProperty(document, 'getAnimations', { - value: jest.fn().mockName('getAnimations'), + value: () => [], + writable: true, }); diff --git a/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap b/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap index 48ff1ad7e7..0c78cafc36 100644 --- a/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap +++ b/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap @@ -33,43 +33,8 @@ exports[`renders a menu from a promise returned from a function 1`] = ` className="loading" >
- - -
+ className="loading-spinner" + />
@@ -202,43 +167,8 @@ exports[`renders an empty menu for a rejected promise 1`] = ` className="loading" >
- - -
+ className="loading-spinner" + /> diff --git a/packages/utils/src/DOMUtils.test.ts b/packages/utils/src/DOMUtils.test.ts index 6ed65c4726..a40bada113 100644 --- a/packages/utils/src/DOMUtils.test.ts +++ b/packages/utils/src/DOMUtils.test.ts @@ -5,10 +5,11 @@ import { } from './DOMUtils'; import TestUtils from './TestUtils'; -const { asMock, createMockProxy } = TestUtils; +const { createMockProxy } = TestUtils; beforeEach(() => { jest.clearAllMocks(); + jest.restoreAllMocks(); expect.hasAssertions(); }); @@ -98,12 +99,14 @@ describe('syncAnimationStartTime', () => { it('should set startTime of all CSSAnimations with given name to given value', () => { expect(cssAnimationA1).toBeInstanceOf(CSSAnimation); - asMock(document.getAnimations).mockReturnValue([ - cssAnimationA1, - cssAnimationA2, - cssAnimationB, - notCSSAnimation, - ]); + jest + .spyOn(document, 'getAnimations') + .mockReturnValue([ + cssAnimationA1, + cssAnimationA2, + cssAnimationB, + notCSSAnimation, + ]); const startTime = 123; From 536fca7c69f3a68b866013efe98048d841e9f25c Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 12:47:11 -0500 Subject: [PATCH 09/20] Small spinner vertical align #1531 --- packages/components/scss/BaseStyleSheet.scss | 3 --- packages/components/src/LoadingSpinner.scss | 8 ++++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/components/scss/BaseStyleSheet.scss b/packages/components/scss/BaseStyleSheet.scss index a2f67e745b..f0bcc0735d 100644 --- a/packages/components/scss/BaseStyleSheet.scss +++ b/packages/components/scss/BaseStyleSheet.scss @@ -177,9 +177,6 @@ span.btn-disabled-wrapper { .fa-layers, .loading-spinner { margin-right: 0.5rem; - // .fa-layers already uses this `vertical-align` to align spinner with - // button text. Setting explicilty so that .loading-spinner aligns the same - vertical-align: -0.125em; } } diff --git a/packages/components/src/LoadingSpinner.scss b/packages/components/src/LoadingSpinner.scss index 6c6931e194..bc9456c093 100644 --- a/packages/components/src/LoadingSpinner.scss +++ b/packages/components/src/LoadingSpinner.scss @@ -16,11 +16,19 @@ margin: 0 auto; width: var(--size); height: var(--size); + + // The original LoadingSpinner used `.fa-layers` to create the spinner icon. + // This vertical-align value was copied from the `.fa-layers` class to avoid + // breaking alignment in existing usages such as `ContextActions` and + // `.btn-spinner` + vertical-align: -0.125em; } .loading-spinner-large { --border-width: 4px; --size: 56px; + + vertical-align: 0; } // Spinning circle with colored border and transparent center. Half of the From 9e81d0c7c10354c885bc345c2706f23c7946991b Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 21 Sep 2023 13:22:45 -0500 Subject: [PATCH 10/20] Gap between loading spinner #1531 --- .../src/console-history/ConsoleHistoryResultInProgress.scss | 1 + .../src/console-history/ConsoleHistoryResultInProgress.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/console/src/console-history/ConsoleHistoryResultInProgress.scss b/packages/console/src/console-history/ConsoleHistoryResultInProgress.scss index ed6ab979d2..270c9cc08f 100644 --- a/packages/console/src/console-history/ConsoleHistoryResultInProgress.scss +++ b/packages/console/src/console-history/ConsoleHistoryResultInProgress.scss @@ -13,6 +13,7 @@ $button-height: ConsoleVariables.$button-height; color: $foreground; font-size: $font-size; font-weight: $font-weight-base; + gap: 4px; margin: $button-vert-margin $button-vert-margin $button-vert-margin 0; height: $button-height; border-radius: $button-height; diff --git a/packages/console/src/console-history/ConsoleHistoryResultInProgress.tsx b/packages/console/src/console-history/ConsoleHistoryResultInProgress.tsx index 92add8fecb..ec0a82f115 100644 --- a/packages/console/src/console-history/ConsoleHistoryResultInProgress.tsx +++ b/packages/console/src/console-history/ConsoleHistoryResultInProgress.tsx @@ -69,7 +69,7 @@ class ConsoleHistoryResultInProgress extends Component< > -  Running... {TimeUtils.formatElapsedTime(elapsed)}  + Running... {TimeUtils.formatElapsedTime(elapsed)}  From a9c0f511ba715624cc6043fb7106d94411accc66 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Mon, 25 Sep 2023 17:38:14 -0500 Subject: [PATCH 12/20] Default animation startTime + comments #1531 --- packages/utils/src/DOMUtils.test.ts | 43 +++++++++++++++-------------- packages/utils/src/DOMUtils.ts | 13 ++++++--- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/packages/utils/src/DOMUtils.test.ts b/packages/utils/src/DOMUtils.test.ts index a40bada113..acdd53cbb7 100644 --- a/packages/utils/src/DOMUtils.test.ts +++ b/packages/utils/src/DOMUtils.test.ts @@ -96,25 +96,26 @@ describe('syncAnimationStartTime', () => { notCSSAnimation.startTime = null; }); - it('should set startTime of all CSSAnimations with given name to given value', () => { - expect(cssAnimationA1).toBeInstanceOf(CSSAnimation); - - jest - .spyOn(document, 'getAnimations') - .mockReturnValue([ - cssAnimationA1, - cssAnimationA2, - cssAnimationB, - notCSSAnimation, - ]); - - const startTime = 123; - - syncAnimationStartTime('animationA', startTime); - - expect(cssAnimationA1.startTime).toBe(startTime); - expect(cssAnimationA2.startTime).toBe(startTime); - expect(cssAnimationB.startTime).toBeNull(); - expect(notCSSAnimation.startTime).toBeNull(); - }); + it.each([undefined, 123])( + 'should set startTime of all CSSAnimations with given name to given value: %s', + startTime => { + expect(cssAnimationA1).toBeInstanceOf(CSSAnimation); + + jest + .spyOn(document, 'getAnimations') + .mockReturnValue([ + cssAnimationA1, + cssAnimationA2, + cssAnimationB, + notCSSAnimation, + ]); + + syncAnimationStartTime('animationA', startTime); + + expect(cssAnimationA1.startTime).toBe(startTime ?? 0); + expect(cssAnimationA2.startTime).toBe(startTime ?? 0); + expect(cssAnimationB.startTime).toBeNull(); + expect(notCSSAnimation.startTime).toBeNull(); + } + ); }); diff --git a/packages/utils/src/DOMUtils.ts b/packages/utils/src/DOMUtils.ts index 56dc0f5b17..9eefc6b7db 100644 --- a/packages/utils/src/DOMUtils.ts +++ b/packages/utils/src/DOMUtils.ts @@ -29,13 +29,18 @@ export function identityExtractHTMLElement( } /** - * Synchronize CSS animations with a given name to the same start time. - * @param animationName - * @param startTime + * Synchronize CSS animations with a given name to the given start time. + * This works because, by default, all animations share the same timeline instance + * with the document, aka. animation.timeline === document.timeline. The `startTime` + * property determines the scheduled time in milliseconds relative to the timeline + * that the animation should begin. + * @param animationName Name of the CSS animation + * @param startTime Start time in milliseconds relative to the animation timeline. + * Default is 0. */ export function syncAnimationStartTime( animationName: string, - startTime: number + startTime = 0 ): void { const animations = document .getAnimations() From 79461e45abfe31caedcfab4da965f950dc4b1b37 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Mon, 25 Sep 2023 17:44:08 -0500 Subject: [PATCH 13/20] Renamed CSS class #1531 --- packages/code-studio/src/styleguide/Tooltips.tsx | 2 +- packages/components/scss/BaseStyleSheet.scss | 2 +- packages/components/src/context-actions/ContextMenu.tsx | 2 +- .../console/src/command-history/CommandHistoryItemTooltip.tsx | 2 +- packages/iris-grid/src/ColumnStatistics.tsx | 2 +- packages/iris-grid/src/IrisGridCopyHandler.tsx | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/code-studio/src/styleguide/Tooltips.tsx b/packages/code-studio/src/styleguide/Tooltips.tsx index 2fc12b59d6..027e8fa9b6 100644 --- a/packages/code-studio/src/styleguide/Tooltips.tsx +++ b/packages/code-studio/src/styleguide/Tooltips.tsx @@ -46,7 +46,7 @@ function Tooltips(): React.ReactElement {
And some icons down here
- + {iconElements}
diff --git a/packages/components/scss/BaseStyleSheet.scss b/packages/components/scss/BaseStyleSheet.scss index 0f237ef43f..76e2868e54 100644 --- a/packages/components/scss/BaseStyleSheet.scss +++ b/packages/components/scss/BaseStyleSheet.scss @@ -181,7 +181,7 @@ span.btn-disabled-wrapper { } .btn-spinner .loading-spinner, -.mimic-fa-layers-vertical-align { +.loading-spinner-vertical-align { // The original LoadingSpinner used `.fa-layers` to create the spinner icon. // This includes a vertical aligment adjustment to center the spinner along // side of other inline content. Copying this value from the `.fa-layers` diff --git a/packages/components/src/context-actions/ContextMenu.tsx b/packages/components/src/context-actions/ContextMenu.tsx index a00cd6b6b2..879743c729 100644 --- a/packages/components/src/context-actions/ContextMenu.tsx +++ b/packages/components/src/context-actions/ContextMenu.tsx @@ -594,7 +594,7 @@ class ContextMenu extends PureComponent { if (pendingItems.length > 0) { pendingElement = (
- +
); } diff --git a/packages/console/src/command-history/CommandHistoryItemTooltip.tsx b/packages/console/src/command-history/CommandHistoryItemTooltip.tsx index 5efb6615dc..01a1ed3b80 100644 --- a/packages/console/src/command-history/CommandHistoryItemTooltip.tsx +++ b/packages/console/src/command-history/CommandHistoryItemTooltip.tsx @@ -195,7 +195,7 @@ export class CommandHistoryItemTooltip extends Component< {hasTimeString ? ( {timeString} ) : ( - + )} diff --git a/packages/iris-grid/src/ColumnStatistics.tsx b/packages/iris-grid/src/ColumnStatistics.tsx index 80c0738b92..3d6976703c 100644 --- a/packages/iris-grid/src/ColumnStatistics.tsx +++ b/packages/iris-grid/src/ColumnStatistics.tsx @@ -258,7 +258,7 @@ class ColumnStatistics extends Component< )} {loading && (
- + Calculating Stats...
)} diff --git a/packages/iris-grid/src/IrisGridCopyHandler.tsx b/packages/iris-grid/src/IrisGridCopyHandler.tsx index 1509ed5b9b..f87b87b1af 100644 --- a/packages/iris-grid/src/IrisGridCopyHandler.tsx +++ b/packages/iris-grid/src/IrisGridCopyHandler.tsx @@ -419,7 +419,7 @@ class IrisGridCopyHandler extends Component< disabled={isFetching} > {isFetching && ( - + )} {copyButtonText} From 9e1a8d6d6f906fe8290577e594826b5e1857cf29 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 26 Sep 2023 08:57:17 -0500 Subject: [PATCH 14/20] Vertical align inline spinners #1531 --- packages/iris-grid/src/PartitionSelectorSearch.tsx | 2 +- packages/iris-grid/src/PendingDataBottomBar.tsx | 2 +- packages/iris-grid/src/sidebar/CustomColumnBuilder.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/iris-grid/src/PartitionSelectorSearch.tsx b/packages/iris-grid/src/PartitionSelectorSearch.tsx index 0b010315bb..24adae3fe6 100644 --- a/packages/iris-grid/src/PartitionSelectorSearch.tsx +++ b/packages/iris-grid/src/PartitionSelectorSearch.tsx @@ -356,7 +356,7 @@ class PartitionSelectorSearch extends Component< )} {isLoading && (
- +  Loading...
)} diff --git a/packages/iris-grid/src/PendingDataBottomBar.tsx b/packages/iris-grid/src/PendingDataBottomBar.tsx index 1a1bc2e01f..5efa55a74c 100644 --- a/packages/iris-grid/src/PendingDataBottomBar.tsx +++ b/packages/iris-grid/src/PendingDataBottomBar.tsx @@ -94,7 +94,7 @@ export function PendingDataBottomBar({ const pendingRowCount = pendingDataMap.size; let commitIcon; if (isSaving) { - commitIcon = ; + commitIcon = ; } else if (wasSuccessShown) { commitIcon = vsPass; } diff --git a/packages/iris-grid/src/sidebar/CustomColumnBuilder.tsx b/packages/iris-grid/src/sidebar/CustomColumnBuilder.tsx index 2794838864..2697048a12 100644 --- a/packages/iris-grid/src/sidebar/CustomColumnBuilder.tsx +++ b/packages/iris-grid/src/sidebar/CustomColumnBuilder.tsx @@ -368,7 +368,7 @@ class CustomColumnBuilder extends Component< > {isCustomColumnApplying && ( - + Applying )} From 7ff52200522c1d8623d7c6de59d5ab60b6ee6909 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 26 Sep 2023 09:01:42 -0500 Subject: [PATCH 15/20] Vertical align spinners #1531 --- packages/auth-plugins/src/LoginForm.tsx | 2 +- packages/code-studio/src/styleguide/Progress.tsx | 2 +- packages/iris-grid/src/sidebar/TableCsvExporter.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/auth-plugins/src/LoginForm.tsx b/packages/auth-plugins/src/LoginForm.tsx index e13114d609..32b5d6c800 100644 --- a/packages/auth-plugins/src/LoginForm.tsx +++ b/packages/auth-plugins/src/LoginForm.tsx @@ -49,7 +49,7 @@ export function LoginForm({ > {isLoggingIn && ( - + Logging in Cancel diff --git a/packages/code-studio/src/styleguide/Progress.tsx b/packages/code-studio/src/styleguide/Progress.tsx index ee9a785e42..cd17d68aab 100644 --- a/packages/code-studio/src/styleguide/Progress.tsx +++ b/packages/code-studio/src/styleguide/Progress.tsx @@ -34,7 +34,7 @@ function Progress(): React.ReactElement { onClick={() => undefined} > - + Connecting Cancel diff --git a/packages/iris-grid/src/sidebar/TableCsvExporter.tsx b/packages/iris-grid/src/sidebar/TableCsvExporter.tsx index ea50c2dd3f..2f93a43ae2 100644 --- a/packages/iris-grid/src/sidebar/TableCsvExporter.tsx +++ b/packages/iris-grid/src/sidebar/TableCsvExporter.tsx @@ -539,7 +539,7 @@ class TableCsvExporter extends Component< > {isDownloading && ( - + Downloading Cancel From 017911f24c3230e9c4d0696a07ede2936a8fcc96 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 26 Sep 2023 10:48:23 -0500 Subject: [PATCH 16/20] Moved loading spinner CSS class #1531 --- packages/components/scss/BaseStyleSheet.scss | 9 --------- packages/components/src/LoadingSpinner.scss | 8 ++++++++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/components/scss/BaseStyleSheet.scss b/packages/components/scss/BaseStyleSheet.scss index 76e2868e54..f0bcc0735d 100644 --- a/packages/components/scss/BaseStyleSheet.scss +++ b/packages/components/scss/BaseStyleSheet.scss @@ -180,15 +180,6 @@ span.btn-disabled-wrapper { } } -.btn-spinner .loading-spinner, -.loading-spinner-vertical-align { - // The original LoadingSpinner used `.fa-layers` to create the spinner icon. - // This includes a vertical aligment adjustment to center the spinner along - // side of other inline content. Copying this value from the `.fa-layers` - // class to avoid breaking alignment of the new spinner. - vertical-align: -0.125em; -} - .btn-link.no-underline, .btn-link.no-underline:hover { text-decoration: none; diff --git a/packages/components/src/LoadingSpinner.scss b/packages/components/src/LoadingSpinner.scss index 6c6931e194..cb46e63c87 100644 --- a/packages/components/src/LoadingSpinner.scss +++ b/packages/components/src/LoadingSpinner.scss @@ -23,6 +23,14 @@ --size: 56px; } +.loading-spinner-vertical-align { + // The original LoadingSpinner used `.fa-layers` to create the spinner icon. + // This includes a vertical aligment adjustment to center the spinner along + // side of other inline content. Copying this value from the `.fa-layers` + // class to avoid breaking alignment of the new spinner. + vertical-align: -0.125em; +} + // Spinning circle with colored border and transparent center. Half of the // circle is the primary color. The other half is the secondary color. .loading-spinner::after { From 4cf22b0e0c669402f3125dfbec6320d96fe3e99e Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 26 Sep 2023 10:56:06 -0500 Subject: [PATCH 17/20] Updated snapshots #1531 --- .../__snapshots__/ContextActions.test.tsx.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap b/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap index 0c78cafc36..5b9825e403 100644 --- a/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap +++ b/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap @@ -33,7 +33,7 @@ exports[`renders a menu from a promise returned from a function 1`] = ` className="loading" >
@@ -167,7 +167,7 @@ exports[`renders an empty menu for a rejected promise 1`] = ` className="loading" >
From 7d42374011879f0cd853cd958a3e28a638357e7f Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 26 Sep 2023 11:00:57 -0500 Subject: [PATCH 18/20] Ran prettier on scss files #1531 --- .../components/scss/bootstrap_overrides.scss | 18 ++++++++++++++---- packages/components/scss/new_variables.scss | 13 ++++++++++--- .../scss/goldenlayout-dark-theme.scss | 8 ++++++-- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/packages/components/scss/bootstrap_overrides.scss b/packages/components/scss/bootstrap_overrides.scss index 5e331576d7..bd05294794 100644 --- a/packages/components/scss/bootstrap_overrides.scss +++ b/packages/components/scss/bootstrap_overrides.scss @@ -95,8 +95,15 @@ $theme-color-interval: 9%; $yiq-contrasted-threshold: 180; // Override fonts -$font-family-sans-serif: 'Fira Sans', -apple-system, blinkmacsystemfont, - 'Segoe UI', 'Roboto', 'Helvetica Neue', arial, sans-serif; //fira sans then native system ui fallbacks +$font-family-sans-serif: + 'Fira Sans', + -apple-system, + blinkmacsystemfont, + 'Segoe UI', + 'Roboto', + 'Helvetica Neue', + arial, + sans-serif; //fira sans then native system ui fallbacks $font-family-monospace: 'Fira Mono', menlo, monaco, consolas, 'Liberation Mono', 'Courier New', monospace; $font-family-base: $font-family-sans-serif; @@ -126,8 +133,11 @@ $box-shadow-900: 0 0.1rem 1rem rgba(0, 0, 0, 45%); //darkest shadow for $black p //Override Btn $btn-border-radius: 4rem; $btn-padding-x: 1.5rem; -$btn-transition: color 0.12s ease-in-out, background-color 0.12s ease-in-out, - border-color 0.12s ease-in-out, box-shadow 0.12s ease-in-out; //default 0.15 is too long +$btn-transition: + color 0.12s ease-in-out, + background-color 0.12s ease-in-out, + border-color 0.12s ease-in-out, + box-shadow 0.12s ease-in-out; //default 0.15 is too long $btn-border-width: 2px; //Override Inputs diff --git a/packages/components/scss/new_variables.scss b/packages/components/scss/new_variables.scss index 65f8733803..7fdfb4eec0 100644 --- a/packages/components/scss/new_variables.scss +++ b/packages/components/scss/new_variables.scss @@ -20,9 +20,16 @@ $ant-thickness: 1px; linear-gradient(to right, $color-2 50%, $color-1 50%), linear-gradient(to bottom, $color-2 50%, $color-1 50%), linear-gradient(to bottom, $color-2 50%, $color-1 50%); - background-size: $ant-size $ant-thickness, $ant-size $ant-thickness, - $ant-thickness $ant-size, $ant-thickness $ant-size; - background-position: 0 top, 0 bottom, left 0, right 0; + background-size: + $ant-size $ant-thickness, + $ant-size $ant-thickness, + $ant-thickness $ant-size, + $ant-thickness $ant-size; + background-position: + 0 top, + 0 bottom, + left 0, + right 0; background-repeat: repeat-x, repeat-x, repeat-y, repeat-y; animation: march 0.5s; animation-timing-function: linear; diff --git a/packages/golden-layout/scss/goldenlayout-dark-theme.scss b/packages/golden-layout/scss/goldenlayout-dark-theme.scss index ab71d74f21..ef6617fb2e 100644 --- a/packages/golden-layout/scss/goldenlayout-dark-theme.scss +++ b/packages/golden-layout/scss/goldenlayout-dark-theme.scss @@ -163,7 +163,9 @@ body:not(.lm_dragging) .lm_header .lm_tab .lm_close_tab:hover { margin: 0; padding: 0 $spacer-1 0 $spacer-2; box-shadow: inset -1px -1px 0 0 $background; // acting as bottom and right border - transition: color $transition, background-color $transition; + transition: + color $transition, + background-color $transition; max-width: 12rem; white-space: nowrap; overflow: hidden; @@ -224,7 +226,9 @@ body:not(.lm_dragging) .lm_header .lm_tab .lm_close_tab:hover { box-shadow: inset -1px 0 0 0 $background; // act as right border only when active // also kills the default shadow, doesn't work well with our design &.lm_focusin { - box-shadow: inset 0 1px $primary, inset -1px 0 0 0 $background; // top focus indicator, right border + box-shadow: + inset 0 1px $primary, + inset -1px 0 0 0 $background; // top focus indicator, right border } } From fdd34ee20593e56c438be4d63e32f987dc93b935 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 27 Sep 2023 09:27:38 -0500 Subject: [PATCH 19/20] Added aria + role attribute. Fixed tests #1531 --- packages/components/src/LoadingSpinner.tsx | 4 ++++ .../__snapshots__/ContextActions.test.tsx.snap | 8 ++++++++ .../command-history/CommandHistoryItemTooltip.test.tsx | 2 +- packages/iris-grid/src/IrisGridCopyHandler.test.tsx | 5 ++--- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/components/src/LoadingSpinner.tsx b/packages/components/src/LoadingSpinner.tsx index be21bc1d5d..9b8f464d4f 100644 --- a/packages/components/src/LoadingSpinner.tsx +++ b/packages/components/src/LoadingSpinner.tsx @@ -21,7 +21,11 @@ function LoadingSpinner({ return (
); } diff --git a/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap b/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap index 5b9825e403..74cf894865 100644 --- a/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap +++ b/packages/components/src/context-actions/__snapshots__/ContextActions.test.tsx.snap @@ -33,7 +33,11 @@ exports[`renders a menu from a promise returned from a function 1`] = ` className="loading" >
@@ -167,7 +171,11 @@ exports[`renders an empty menu for a rejected promise 1`] = ` className="loading" >
diff --git a/packages/console/src/command-history/CommandHistoryItemTooltip.test.tsx b/packages/console/src/command-history/CommandHistoryItemTooltip.test.tsx index d28aa1d7d3..15fcab1c45 100644 --- a/packages/console/src/command-history/CommandHistoryItemTooltip.test.tsx +++ b/packages/console/src/command-history/CommandHistoryItemTooltip.test.tsx @@ -92,7 +92,7 @@ describe('different command results', () => { it('shows loading spinner while waiting for updates', () => { expect(commandHistoryStorage.listenItem).toHaveBeenCalled(); - expect(screen.getAllByRole('img', { hidden: true }).length).toBe(2); + screen.getByRole('progressbar', { hidden: true }); unmount(); diff --git a/packages/iris-grid/src/IrisGridCopyHandler.test.tsx b/packages/iris-grid/src/IrisGridCopyHandler.test.tsx index 8be7d23b1b..d8b9ebf546 100644 --- a/packages/iris-grid/src/IrisGridCopyHandler.test.tsx +++ b/packages/iris-grid/src/IrisGridCopyHandler.test.tsx @@ -71,9 +71,8 @@ it('copies immediately if less than 10,000 rows of data', async () => { const copyOperation = makeCopyOperation(ranges); const model = makeModel(); mountCopySelection({ copyOperation, model }); - - expect(screen.getAllByRole('img', { hidden: true }).length).toBe(2); - expect(screen.getByText('Fetching 10,000 rows for clipboard...')); + screen.getByRole('progressbar', { hidden: true }); + screen.getByText('Fetching 10,000 rows for clipboard...'); expect(model.textSnapshot).toHaveBeenCalled(); await waitFor(() => From 77480a83f3238c8806b1740207ef31cdeb79ff55 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 27 Sep 2023 10:03:14 -0500 Subject: [PATCH 20/20] Fixed spinner tests #1531 --- .../src/panels/ChartPanel.test.tsx | 7 +++---- .../src/panels/IrisGridPanel.test.tsx | 9 ++++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/dashboard-core-plugins/src/panels/ChartPanel.test.tsx b/packages/dashboard-core-plugins/src/panels/ChartPanel.test.tsx index 62761aa1ce..09a53f576e 100644 --- a/packages/dashboard-core-plugins/src/panels/ChartPanel.test.tsx +++ b/packages/dashboard-core-plugins/src/panels/ChartPanel.test.tsx @@ -20,7 +20,7 @@ jest.mock('@deephaven/dashboard', () => ({ }, })); -const MockChart = jest.fn(() => null); +const MockChart: React.FC & jest.Mock = jest.fn(() => null); jest.mock('@deephaven/chart', () => { const { forwardRef } = jest.requireActual('react'); @@ -106,9 +106,8 @@ function callErrorFunction() { function expectLoading(container) { expect( - container.querySelector("[data-icon='circle-large']") + container.querySelector('[role=progressbar].loading-spinner-large') ).toBeInTheDocument(); - expect(container.querySelector("[data-icon='loading']")).toBeInTheDocument(); } function expectNotLoading(container) { @@ -116,7 +115,7 @@ function expectNotLoading(container) { container.querySelector("[data-icon='outline']") ).not.toBeInTheDocument(); expect( - container.querySelector("[data-icon='loading']") + container.querySelector('[role=progressbar].loading-spinner-large') ).not.toBeInTheDocument(); } diff --git a/packages/dashboard-core-plugins/src/panels/IrisGridPanel.test.tsx b/packages/dashboard-core-plugins/src/panels/IrisGridPanel.test.tsx index 00c77c7c79..6c18901525 100644 --- a/packages/dashboard-core-plugins/src/panels/IrisGridPanel.test.tsx +++ b/packages/dashboard-core-plugins/src/panels/IrisGridPanel.test.tsx @@ -9,7 +9,7 @@ import type { Container } from '@deephaven/golden-layout'; import { Workspace } from '@deephaven/redux'; import { IrisGridPanel } from './IrisGridPanel'; -const MockIrisGrid = jest.fn(() => null); +const MockIrisGrid: React.FC & jest.Mock = jest.fn(() => null); jest.mock('@deephaven/iris-grid', () => { const { forwardRef } = jest.requireActual('react'); @@ -84,10 +84,9 @@ function makeIrisGridPanelWrapper( async function expectLoading(container) { await waitFor(() => expect( - container.querySelector("[data-icon='circle-large']") + container.querySelector('[role=progressbar].loading-spinner-large') ).toBeInTheDocument() ); - expect(container.querySelector("[data-icon='loading']")).toBeInTheDocument(); } async function expectNotLoading(container) { @@ -97,7 +96,7 @@ async function expectNotLoading(container) { ).not.toBeInTheDocument() ); expect( - container.querySelector("[data-icon='loading']") + container.querySelector('[role=progressbar].loading-spinner-large') ).not.toBeInTheDocument(); } @@ -127,7 +126,7 @@ it('shows the loading spinner until grid is ready', async () => { const tablePromise = Promise.resolve(table); const makeModel = makeMakeModel(tablePromise); - expect.assertions(6); + expect.assertions(4); const { container } = makeIrisGridPanelWrapper(makeModel); await expectLoading(container);