Spinner: Fix so that the size property is correctly applied (#77135)

* correctly apply spinner sizes, refactor to have standard sizes * better prop definitions so constant values show in IDEs * 12 is xs, not sm
2024-11-26 02:40:26 -06:00 · 2023-10-25 15:39:32 +01:00 · 2023-10-25 15:39:32 +01:00 · 5f2fd8935d
commit 5f2fd8935d
parent b88b206ee2
14 changed files with 105 additions and 35 deletions
--- a/packages/grafana-ui/src/components/Icon/Icon.tsx
+++ b/packages/grafana-ui/src/components/Icon/Icon.tsx
@ -58,14 +58,7 @@ export const Icon = React.forwardRef<SVGElement, IconProps>(
        width={svgWid}
        height={svgHgt}
        title={title}
-        className={cx(
-          styles.icon,
-          {
-            'fa-spin': iconName === 'spinner',
-          },
-          className,
-          type === 'mono' ? { [styles.orange]: name === 'favorite' } : ''
-        )}
+        className={cx(styles.icon, className, type === 'mono' ? { [styles.orange]: name === 'favorite' } : '')}
        style={style}
        {...rest}
      />
--- a/packages/grafana-ui/src/components/Spinner/Spinner.story.tsx
+++ b/packages/grafana-ui/src/components/Spinner/Spinner.story.tsx
@ -49,7 +49,7 @@ export const Basic: Story<StoryProps> = (args) => {
 Basic.args = {
  backgroundColor: 'white',
  color: 'red',
-  size: 34,
+  size: 'xl',
  withStyle: false,
 };

--- a/packages/grafana-ui/src/components/Spinner/Spinner.tsx
+++ b/packages/grafana-ui/src/components/Spinner/Spinner.tsx
@ -1,34 +1,108 @@
 import { cx, css } from '@emotion/css';
 import React from 'react';
+import SVG from 'react-inlinesvg';

-import { stylesFactory } from '../../themes';
+import { GrafanaTheme2 } from '@grafana/data';
+
+import { useStyles2 } from '../../themes';
+import { IconSize, isIconSize } from '../../types';
 import { Icon } from '../Icon/Icon';
+import { getIconRoot, getIconSubDir } from '../Icon/utils';

-const getStyles = stylesFactory((size: number | string, inline: boolean) => {
-  return css([
-    {
-      fontSize: typeof size === 'string' ? size : `${size}px`,
-    },
-    inline && { display: 'inline-block' },
-  ]);
-});
-
-export type Props = {
+export interface Props {
  className?: string;
  style?: React.CSSProperties;
  iconClassName?: string;
  inline?: boolean;
+  size?: IconSize;
+}
+
+/**
+ * @deprecated
+ * use a predefined size, e.g. 'md' or 'lg' instead
+ */
+interface PropsWithDeprecatedSize extends Omit<Props, 'size'> {
  size?: number | string;
-};
+}

 /**
 * @public
 */
-export const Spinner = ({ className, inline = false, iconClassName, style, size = 16 }: Props) => {
-  const styles = getStyles(size, inline);
+export const Spinner = ({
+  className,
+  inline = false,
+  iconClassName,
+  style,
+  size = 'md',
+}: Props | PropsWithDeprecatedSize) => {
+  const styles = useStyles2(getStyles);
+
+  const deprecatedStyles = useStyles2(getDeprecatedStyles, size);
+
+  // this entire if statement is handling the deprecated size prop
+  // TODO remove once we fully remove the deprecated type
+  if (typeof size !== 'string' || !isIconSize(size)) {
+    const iconRoot = getIconRoot();
+    const iconName = 'spinner';
+    const subDir = getIconSubDir(iconName, 'default');
+    const svgPath = `${iconRoot}${subDir}/${iconName}.svg`;
+    return (
+      <div
+        data-testid="Spinner"
+        style={style}
+        className={cx(
+          {
+            [styles.inline]: inline,
+          },
+          deprecatedStyles.wrapper,
+          className
+        )}
+      >
+        <SVG
+          src={svgPath}
+          width={size}
+          height={size}
+          className={cx('fa-spin', deprecatedStyles.icon, className)}
+          style={style}
+        />
+      </div>
+    );
+  }
+
  return (
-    <div data-testid="Spinner" style={style} className={cx(styles, className)}>
-      <Icon className={cx('fa-spin', iconClassName)} name="spinner" aria-label="loading spinner" />
+    <div
+      data-testid="Spinner"
+      style={style}
+      className={cx(
+        {
+          [styles.inline]: inline,
+        },
+        className
+      )}
+    >
+      <Icon className={cx('fa-spin', iconClassName)} name="spinner" size={size} aria-label="loading spinner" />
    </div>
  );
 };
+
+const getStyles = (theme: GrafanaTheme2) => ({
+  inline: css({
+    display: 'inline-block',
+  }),
+});
+
+// TODO remove once we fully remove the deprecated type
+const getDeprecatedStyles = (theme: GrafanaTheme2, size: number | string) => ({
+  wrapper: css({
+    fontSize: typeof size === 'string' ? size : `${size}px`,
+  }),
+  icon: css({
+    display: 'inline-block',
+    fill: 'currentColor',
+    flexShrink: 0,
+    label: 'Icon',
+    // line-height: 0; is needed for correct icon alignment in Safari
+    lineHeight: 0,
+    verticalAlign: 'middle',
+  }),
+});
--- a/packages/grafana-ui/src/types/icon.ts
+++ b/packages/grafana-ui/src/types/icon.ts
@ -8,6 +8,9 @@ export { toIconName } from '@grafana/data';

 export type IconType = 'mono' | 'default' | 'solid';
 export type IconSize = ComponentSize | 'xl' | 'xxl' | 'xxxl';
+export const isIconSize = (value: string): value is IconSize => {
+  return ['xs', 'sm', 'md', 'lg', 'xl', 'xxl', 'xxxl'].includes(value);
+};

 // function remains for backwards compatibility
 export const getAvailableIcons = () => Object.keys(availableIconsIndex);
--- a/public/app/core/components/RolePicker/RolePicker.tsx
+++ b/public/app/core/components/RolePicker/RolePicker.tsx
@ -156,7 +156,7 @@ export const RolePicker = ({
    return (
      <div style={{ maxWidth: widthPx || maxWidth, width: widthPx }}>
        <span>Loading...</span>
-        <Spinner size={16} inline className={styles.loadingSpinner} />
+        <Spinner inline className={styles.loadingSpinner} />
      </div>
    );
  }
--- a/public/app/features/alerting/unified/components/rules/CloudRules.tsx
+++ b/public/app/features/alerting/unified/components/rules/CloudRules.tsx
@ -77,7 +77,7 @@ export const CloudRules = ({ namespaces, expandAll }: Props) => {

      {!hasDataSourcesConfigured && <p>There are no Prometheus or Loki data sources configured.</p>}
      {hasDataSourcesConfigured && !hasDataSourcesLoading && !hasNamespaces && <p>No rules found.</p>}
-      {!hasSomeResults && hasDataSourcesLoading && <Spinner size={24} className={styles.spinner} />}
+      {!hasSomeResults && hasDataSourcesLoading && <Spinner size="xl" className={styles.spinner} />}

      <Pagination
        className={styles.pagination}
--- a/public/app/features/alerting/unified/components/rules/GrafanaRules.tsx
+++ b/public/app/features/alerting/unified/components/rules/GrafanaRules.tsx
@ -62,7 +62,7 @@ export const GrafanaRules = ({ namespaces, expandAll }: Props) => {
        />
      ))}
      {hasResult && namespacesFormat?.length === 0 && <p>No rules found.</p>}
-      {!hasResult && loading && <Spinner size={24} className={styles.spinner} />}
+      {!hasResult && loading && <Spinner size="xl" className={styles.spinner} />}
      <Pagination
        className={styles.pagination}
        currentPage={page}
--- a/public/app/features/dashboard/components/GenAI/GenAIButton.tsx
+++ b/public/app/features/dashboard/components/GenAI/GenAIButton.tsx
@ -174,7 +174,7 @@ export const GenAIButton = ({

  return (
    <div className={styles.wrapper}>
-      {isFirstHistoryEntry && <Spinner size={14} />}
+      {isFirstHistoryEntry && <Spinner size="sm" />}
      {!hasHistory && (
        <Tooltip
          show={error ? undefined : false}
--- a/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/ConfigPublicDashboard/SettingsSummary.tsx
+++ b/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/ConfigPublicDashboard/SettingsSummary.tsx
@ -23,7 +23,7 @@ export function SettingsSummary({

  return isDataLoading ? (
    <div className={cx(styles.summaryWrapper, className)}>
-      <Spinner className={styles.summary} inline={true} size={14} />
+      <Spinner className={styles.summary} inline={true} size="sm" />
    </div>
  ) : (
    <div className={cx(styles.summaryWrapper, className)}>
--- a/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboard.tsx
+++ b/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboard.tsx
@ -20,7 +20,7 @@ const Loader = () => {
    <HorizontalGroup className={styles.loadingContainer}>
      <>
        Loading configuration
-        <Spinner size={20} className={styles.spinner} />
+        <Spinner size="lg" className={styles.spinner} />
      </>
    </HorizontalGroup>
  );
--- a/public/app/features/manage-dashboards/DashboardImportPage.tsx
+++ b/public/app/features/manage-dashboards/DashboardImportPage.tsx
@ -238,7 +238,7 @@ class UnthemedDashboardImport extends PureComponent<Props> {
          {loadingState === LoadingState.Loading && (
            <VerticalGroup justify="center">
              <HorizontalGroup justify="center">
-                <Spinner size={32} />
+                <Spinner size="xxl" />
              </HorizontalGroup>
            </VerticalGroup>
          )}
--- a/public/app/features/plugins/sql/components/query-editor-raw/QueryValidator.tsx
+++ b/public/app/features/plugins/sql/components/query-editor-raw/QueryValidator.tsx
@ -79,7 +79,7 @@ export function QueryValidator({ db, query, onValidate, range }: QueryValidatorP
    <>
      {state.loading && (
        <div className={styles.info}>
-          <Spinner inline={true} size={12} /> Validating query...
+          <Spinner inline={true} size="xs" /> Validating query...
        </div>
      )}
      {!state.loading && state.value && (
--- a/public/app/features/variables/inspect/VariablesUnknownTable.tsx
+++ b/public/app/features/variables/inspect/VariablesUnknownTable.tsx
@ -58,7 +58,7 @@ export function VariablesUnknownTable({ variables, dashboard }: VariablesUnknown
          <VerticalGroup justify="center">
            <HorizontalGroup justify="center">
              <span>Loading...</span>
-              <Spinner size={16} />
+              <Spinner />
            </HorizontalGroup>
          </VerticalGroup>
        )}
--- a/public/app/plugins/panel/gettingstarted/GettingStarted.tsx
+++ b/public/app/plugins/panel/gettingstarted/GettingStarted.tsx
@ -86,7 +86,7 @@ export class GettingStarted extends PureComponent<PanelProps, State> {
        {!checksDone ? (
          <div className={styles.loading}>
            <div className={styles.loadingText}>Checking completed setup steps</div>
-            <Spinner size={24} inline />
+            <Spinner size="xl" inline />
          </div>
        ) : (
          <>