Skip to content

Commit

Permalink
fix!: CSS based loading spinner (#1532)
Browse files Browse the repository at this point in the history
- Swapped our LoadingSpinner implementation with a simpler html / CSS
only spinner
- Fixes a layout shift after font load caused by `font-size:
$iris-panel-icon-font-size` in `.message-icon` of LoadingOverlay
- Fixes loading spinner backtracking due to multiple instances of
spinners

**Regression Tests Performed**
- LoadingOverlay - I added a synthetic wait to PluginUtils right after
the manifest loading. Loading overlay is now seamless. No layout shift
or backtracking

Verified the following look as they did before

Community
- Styleguide indeterminate and progress spinners 
- Styleguide context menu spinners
- Console "Running" spinner
- AppMainContainer disconnection spinner
- Command history tooltip elapsed spinner
- Advanced filter spinner
- Column statistics spinner
- IrisGridCopyHandler slight adjustment such that spinner + text are
centered
- PartitionSelectorSearch spinner
- Commit button in PendingDataBottomBar
- Custom column builder "applying" spinner
- Table CSV exporter spinner

Enterprise
- Enterprise login spinner

```
  await new Promise(resolve => {
      setTimeout(resolve, 2000);
  });
```

fixes #1531

BREAKING CHANGE: Inline LoadingSpinner instances will need to be
decorated with `className="loading-spinner-vertical-align"` for vertical
alignment to work as before
  • Loading branch information
bmingles authored Sep 28, 2023
1 parent 49723ec commit f06fbb0
Show file tree
Hide file tree
Showing 31 changed files with 267 additions and 129 deletions.
11 changes: 11 additions & 0 deletions jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -57,3 +63,8 @@ Object.defineProperty(document, 'fonts', {
ready: Promise.resolve(),
},
});

Object.defineProperty(document, 'getAnimations', {
value: () => [],
writable: true,
});
2 changes: 1 addition & 1 deletion packages/auth-plugins/src/LoginForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function LoginForm({
>
{isLoggingIn && (
<span>
<LoadingSpinner />
<LoadingSpinner className="loading-spinner-vertical-align" />
<span className="btn-normal-content">Logging in</span>
<span className="btn-hover-content">Cancel</span>
</span>
Expand Down
2 changes: 1 addition & 1 deletion packages/code-studio/src/styleguide/Progress.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function Progress(): React.ReactElement {
onClick={() => undefined}
>
<span>
<LoadingSpinner />
<LoadingSpinner className="loading-spinner-vertical-align" />
<span className="btn-normal-content">Connecting</span>
<span className="btn-hover-content">Cancel</span>
</span>
Expand Down
2 changes: 1 addition & 1 deletion packages/code-studio/src/styleguide/Tooltips.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function Tooltips(): React.ReactElement {
<hr />
<div>And some icons down here</div>
<div>
<LoadingSpinner />
<LoadingSpinner className="loading-spinner-vertical-align" />
{iconElements}
</div>
</Tooltip>
Expand Down
25 changes: 19 additions & 6 deletions packages/components/scss/BaseStyleSheet.scss
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ span.btn-disabled-wrapper {

.btn-spinner {
padding: $btn-padding-y 1rem;
.fa-layers {
.fa-layers,
.loading-spinner {
margin-right: 0.5rem;
}
}
Expand Down Expand Up @@ -255,7 +256,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;
}

Expand Down Expand Up @@ -712,7 +715,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
}
}
Expand Down Expand Up @@ -746,7 +750,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%);
Expand Down Expand Up @@ -835,11 +841,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;
}
}
Expand Down
18 changes: 14 additions & 4 deletions packages/components/scss/bootstrap_overrides.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
13 changes: 10 additions & 3 deletions packages/components/scss/new_variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions packages/components/src/LoadingOverlay.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@ $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 {
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: inherit;
}
Expand Down
8 changes: 7 additions & 1 deletion packages/components/src/LoadingOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ function LoadingOverlay({
dataTestId != null ? `${dataTestId}-message` : undefined;
const spinnerTestId =
dataTestId != null ? `${dataTestId}-spinner` : undefined;

return (
<CSSTransition
in={Boolean(errorMessage) || !isLoaded || isLoading}
Expand All @@ -45,7 +46,12 @@ function LoadingOverlay({
>
<div className="message-content">
<div className="message-icon">
{isLoading && <LoadingSpinner data-testid={spinnerTestId} />}
{isLoading && (
<LoadingSpinner
className="loading-spinner-large"
data-testid={spinnerTestId}
/>
)}
{!isLoading && errorMessage != null && (
<FontAwesomeIcon icon={vsWarning} />
)}
Expand Down
56 changes: 53 additions & 3 deletions packages/components/src/LoadingSpinner.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,56 @@
/* stylelint-disable alpha-value-notation */
.loading-spinner {
--primary-color: var(
--dh-loading-spinner-primary-color,
var(--dh-accent-color, #4c7dee)
);
--secondary-color: var(
--dh-loading-spinner-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;
}

.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 {
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);
}
}
22 changes: 14 additions & 8 deletions packages/components/src/LoadingSpinner.tsx
Original file line number Diff line number Diff line change
@@ -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 = {
Expand All @@ -13,14 +12,21 @@ 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 (
<div
className={classNames('loading-spinner fa-layers', className)}
className={classNames('loading-spinner', className)}
aria-label="Loading..."
aria-valuemin={0}
aria-valuemax={100}
data-testid={dataTestId}
>
<FontAwesomeIcon icon={vsCircleLarge} className="text-white-50" />
<FontAwesomeIcon icon={vsLoading} className="text-primary" spin />
</div>
role="progressbar"
/>
);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/context-actions/ContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ class ContextMenu extends PureComponent<ContextMenuProps, ContextMenuState> {
if (pendingItems.length > 0) {
pendingElement = (
<div className="loading">
<LoadingSpinner />
<LoadingSpinner className="loading-spinner-vertical-align" />
</div>
);
}
Expand Down
Loading

0 comments on commit f06fbb0

Please sign in to comment.