fix(Spinner): Add min height to Spinner container (#505)

* fix(Spinner): Add min height to Spinner container

Previously the spinner's parent container was not taking the height of its children because of the negative margins. A min height matching the text line height fixes this issue

* Remove the now-unnecessary 1px push hack in PreviewCard

* Fixing other vertical-align issues

* fix(Spinner): update Spinner snapshots to match fixed margins

* Update suggestionslist visual diffs

Author: hannakohxt

config/styleguide-visual/reference/PreviewCard_loading_state_desktop.png

@@ -113,7 +113,7 @@ export class PreviewCard extends React.Component<PreviewCardProps & Customizable
 
     if (isLoading && !this.hasProgress()) {
       return (
-        <Block textColor={TextColor.METADATA} push={1}>
+        <Block textColor={TextColor.METADATA}>
           <Spinner text={loadingText as string} size={SpinnerSize.XSMALL} color={SpinnerColor.METADATA} />
         </Block>
       );

config/styleguide-visual/reference/Spinner_centered_desktop.png

@@ -130,7 +130,6 @@ exports[`<PreviewCard /> when loading matches its snapshot 1`] = `
         }
     metadataContent={
       <CustomizedBlock
-        push={1}
         textColor="metadata"
       >
         <CustomizedSpinner

config/styleguide-visual/reference/Spinner_with_invisible_text_desktop.png

@@ -5,30 +5,37 @@ import { memoizeFunction } from 'office-ui-fabric-react/lib/Utilities';
 import { SpinnerColor, SpinnerSize } from './Spinner.types';
 import { getGutterValue } from '../../util/styles/gutters';
 import { ITheme } from '../Customizer';
+import { getFont } from '../../util/styles/fonts';
+import { TextSize } from '../Text';
 
 export interface SpinnerClassNames {
   root: string;
   label: string;
 }
 
 const labelMargins = {
-  [SpinnerSize.XSMALL]: `-1px 0 0 ${getGutterValue(1.5, true)}`,
-  [SpinnerSize.SMALL]: `-1px 0 0 ${getGutterValue(2, true)}`,
-  [SpinnerSize.MEDIUM]: `-2px 0 0 ${getGutterValue(2, true)}`,
+  [SpinnerSize.XSMALL]: `0 0 0 ${getGutterValue(1.5, true)}`,
+  [SpinnerSize.SMALL]: `0 0 0 ${getGutterValue(2, true)}`,
+  [SpinnerSize.MEDIUM]: `-1px 0 0 ${getGutterValue(2, true)}`,
   [SpinnerSize.LARGE]: `${getGutterValue(1, true)} 0 0`,
 };
 export interface SpinnerClassNameProps {
   size: SpinnerSize;
   isCentered: boolean;
+  theme: ITheme;
+  textSize: TextSize;
 }
 
 export const getClassNames = memoizeFunction((classNameProps: SpinnerClassNameProps): SpinnerClassNames => {
-  const { size, isCentered } = classNameProps;
+  const { size, isCentered, theme, textSize } = classNameProps;
+  const font = getFont(textSize, theme);
+
   return mergeStyleSets({
     root: {
       display: 'flex',
       alignItems: 'center',
       flexDirection: size === SpinnerSize.LARGE ? 'column' : 'row',
+      minHeight: font.lineHeight,
       justifyContent: isCentered ? 'center' : undefined,
     },
     label: {

config/styleguide-visual/reference/Spinner_with_visible_text_desktop.png

@@ -39,13 +39,22 @@ const fabricSpinnerSizes = {
  */
 export class Spinner extends React.Component<SpinnerProps & CustomizableComponentProps> {
   public render() {
-    const { text, hideText, color = SpinnerColor.LIGHT, size = SpinnerSize.MEDIUM, isCentered = false } = this.props;
-    const classNames = getClassNames({ size, isCentered });
+    const {
+      theme = defaultTheme,
+      text,
+      hideText,
+      color = SpinnerColor.LIGHT,
+      size = SpinnerSize.MEDIUM,
+      isCentered = false,
+    } = this.props;
+
+    const textSize = labelSizes[size];
+    const classNames = getClassNames({ size, isCentered, theme, textSize });
 
     const label = hideText ? (
       <ScreenreaderText>{text}</ScreenreaderText>
     ) : (
-      <Text className={classNames.label} color={labelColors[color]} size={labelSizes[size]}>
+      <Text className={classNames.label} color={labelColors[color]} size={textSize}>
         {text}
       </Text>
     );

config/styleguide-visual/reference/SuggestionsList_when_loading_desktop.png

@@ -9,6 +9,7 @@ exports[`<Spinner /> when centered matches its snapshot 1`] = `
         display: flex;
         flex-direction: row;
         justify-content: center;
+        min-height: 2.0rem;
       }
 >
   <StyledCustomizedSpinner
@@ -28,7 +29,7 @@ exports[`<Spinner /> when centered matches its snapshot 1`] = `
           margin-bottom: 0;
           margin-left: 0.8rem;
           margin-right: 0;
-          margin-top: -2px;
+          margin-top: -1px;
         }
     color="primary"
     size="mediumSub"
@@ -46,6 +47,7 @@ exports[`<Spinner /> with additional className matches its snapshot 1`] = `
         align-items: center;
         display: flex;
         flex-direction: row;
+        min-height: 2.0rem;
       }
 >
   <StyledCustomizedSpinner
@@ -65,7 +67,7 @@ exports[`<Spinner /> with additional className matches its snapshot 1`] = `
           margin-bottom: 0;
           margin-left: 0.8rem;
           margin-right: 0;
-          margin-top: -2px;
+          margin-top: -1px;
         }
     color="primary"
     size="mediumSub"
@@ -83,6 +85,7 @@ exports[`<Spinner /> with dark color matches its snapshot 1`] = `
         align-items: center;
         display: flex;
         flex-direction: row;
+        min-height: 2.0rem;
       }
 >
   <StyledCustomizedSpinner
@@ -102,7 +105,7 @@ exports[`<Spinner /> with dark color matches its snapshot 1`] = `
           margin-bottom: 0;
           margin-left: 0.8rem;
           margin-right: 0;
-          margin-top: -2px;
+          margin-top: -1px;
         }
     color="white"
     size="mediumSub"
@@ -120,6 +123,7 @@ exports[`<Spinner /> with invisible text matches its snapshot 1`] = `
         align-items: center;
         display: flex;
         flex-direction: row;
+        min-height: 2.0rem;
       }
 >
   <StyledCustomizedSpinner
@@ -146,6 +150,7 @@ exports[`<Spinner /> with large size matches its snapshot 1`] = `
         align-items: center;
         display: flex;
         flex-direction: column;
+        min-height: 2.0rem;
       }
 >
   <StyledCustomizedSpinner
@@ -184,6 +189,7 @@ exports[`<Spinner /> with metadata color matches its snapshot 1`] = `
         align-items: center;
         display: flex;
         flex-direction: row;
+        min-height: 2.0rem;
       }
 >
   <StyledCustomizedSpinner
@@ -203,7 +209,7 @@ exports[`<Spinner /> with metadata color matches its snapshot 1`] = `
           margin-bottom: 0;
           margin-left: 0.8rem;
           margin-right: 0;
-          margin-top: -2px;
+          margin-top: -1px;
         }
     color="metadata"
     size="mediumSub"
@@ -221,6 +227,7 @@ exports[`<Spinner /> with visible text matches its snapshot 1`] = `
         align-items: center;
         display: flex;
         flex-direction: row;
+        min-height: 2.0rem;
       }
 >
   <StyledCustomizedSpinner
@@ -240,7 +247,7 @@ exports[`<Spinner /> with visible text matches its snapshot 1`] = `
           margin-bottom: 0;
           margin-left: 0.8rem;
           margin-right: 0;
-          margin-top: -2px;
+          margin-top: -1px;
         }
     color="primary"
     size="mediumSub"
@@ -258,6 +265,7 @@ exports[`<Spinner /> with xSmall size matches its snapshot 1`] = `
         align-items: center;
         display: flex;
         flex-direction: row;
+        min-height: 1.6rem;
       }
 >
   <StyledCustomizedSpinner
@@ -278,7 +286,7 @@ exports[`<Spinner /> with xSmall size matches its snapshot 1`] = `
           margin-bottom: 0;
           margin-left: 0.6rem;
           margin-right: 0;
-          margin-top: -1px;
+          margin-top: 0;
         }
     color="primary"
     size="small"

config/styleguide-visual/reference/SuggestionsList_with_results_when_loading_desktop.png

@@ -2,7 +2,7 @@
 import { TextSize, TextColor } from './Text.types';
 import { memoizeFunction } from 'office-ui-fabric-react/lib/Utilities';
 import { mergeStyleSets, ITheme } from 'office-ui-fabric-react/lib/Styling';
-import { ellipsisStyle, fontWeights, textColors, verticalAligns } from '../../util/styles/fonts';
+import { ellipsisStyle, fontWeights, getFont, textColors, verticalAligns } from '../../util/styles/fonts';
 import { PaletteColor } from '../../util/colors';
 
 export interface TextClassNameProps {
@@ -16,7 +16,7 @@ export interface TextClassNameProps {
 }
 export const getClassNames = memoizeFunction((classNameProps: TextClassNameProps) => {
   const { size, maxWidth, bold, uppercase, color, backgroundColor, theme } = classNameProps;
-  const font = size ? theme.fonts[size === TextSize.MEDIUM_SUB ? 'smallPlus' : size] : undefined;
+  const font = size ? getFont(size, theme) : undefined;
 
   return mergeStyleSets({
     root: {

src/components/PreviewCard/PreviewCard.tsx

@@ -12,6 +12,9 @@ export const verticalAligns: Record<TextSize, string> = {
   xxLarge: '-0.4rem',
 };
 
+export const getFont = (size: TextSize, theme: ITheme): IRawStyle =>
+  theme.fonts[size === TextSize.MEDIUM_SUB ? 'smallPlus' : size];
+
 export const textColors = (theme: ITheme): Record<TextColor, string> => ({
   primary: theme.semanticColors.bodyText,
   secondary: theme.palette.neutralPrimaryAlt,