Replace flexbox gap property with better supported techniques (#1526)

* replaces flex-gap usage in components with different techniques * cleans up silly mistakes * adds changeset
primer · Oct 27, 2021 · 1378e77 · 1378e77
1 parent 1afc3f7
commit 1378e77
Show file tree

Hide file tree

Showing 7 changed files with 3,490 additions and 3,239 deletions.
diff --git a/.changeset/silent-lions-prove.md b/.changeset/silent-lions-prove.md
@@ -0,0 +1,5 @@
+---
+'@primer/components': patch
+---
+
+replaces flexbox gap usage with margins
diff --git a/src/TextInputWithTokens.tsx b/src/TextInputWithTokens.tsx
@@ -1,4 +1,4 @@
-import React, {FocusEventHandler, KeyboardEventHandler, useRef, useState} from 'react'
+import React, {FocusEventHandler, KeyboardEventHandler, RefObject, useRef, useState} from 'react'
 import {omit} from '@styled-system/props'
 import {FocusKeys} from './behaviors/focusZone'
 import {useCombinedRefs} from './hooks/useCombinedRefs'
@@ -184,16 +184,7 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
       minWidth={minWidthProp}
       maxWidth={maxWidthProp}
       variant={variantProp}
-      ref={containerRef}
       sx={{
-        alignItems: 'center',
-        flexWrap: preventTokenWrapping ? 'nowrap' : 'wrap',
-        gap: '0.25rem',
-
-        '> *': {
-          flexShrink: 0
-        },
-
         ...(block
           ? {
               display: 'flex',
@@ -218,40 +209,58 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
       }}
     >
       <Box
+        ref={containerRef as RefObject<HTMLDivElement>}
+        display="flex"
         sx={{
-          order: 1,
-          flexGrow: 1
+          alignItems: 'center',
+          flexWrap: preventTokenWrapping ? 'nowrap' : 'wrap',
+          marginLeft: '-0.25rem',
+          marginBottom: '-0.25rem',
+          flexGrow: 1,
+
+          '> *': {
+            flexShrink: 0,
+            marginLeft: '0.25rem',
+            marginBottom: '0.25rem'
+          }
         }}
       >
-        {IconComponent && <IconComponent className="TextInput-icon" />}
-        <UnstyledTextInput
-          ref={combinedInputRef}
-          disabled={disabled}
-          onFocus={handleInputFocus}
-          onKeyDown={handleInputKeyDown}
-          type="text"
-          sx={{height: '100%'}}
-          {...inputPropsRest}
-        />
+        <Box
+          sx={{
+            order: 1,
+            flexGrow: 1
+          }}
+        >
+          {IconComponent && <IconComponent className="TextInput-icon" />}
+          <UnstyledTextInput
+            ref={combinedInputRef}
+            disabled={disabled}
+            onFocus={handleInputFocus}
+            onKeyDown={handleInputKeyDown}
+            type="text"
+            sx={{height: '100%'}}
+            {...inputPropsRest}
+          />
+        </Box>
+        {tokens.length && TokenComponent
+          ? tokens.map(({id, ...tokenRest}, i) => (
+              <TokenComponent
+                key={id}
+                onFocus={handleTokenFocus(i)}
+                onBlur={handleTokenBlur}
+                onKeyUp={handleTokenKeyUp}
+                isSelected={selectedTokenIndex === i}
+                onRemove={() => {
+                  handleTokenRemove(id)
+                }}
+                hideRemoveButton={hideTokenRemoveButtons}
+                size={size}
+                tabIndex={0}
+                {...tokenRest}
+              />
+            ))
+          : null}
       </Box>
-      {tokens.length && TokenComponent
-        ? tokens.map(({id, ...tokenRest}, i) => (
-            <TokenComponent
-              key={id}
-              onFocus={handleTokenFocus(i)}
-              onBlur={handleTokenBlur}
-              onKeyUp={handleTokenKeyUp}
-              isSelected={selectedTokenIndex === i}
-              onRemove={() => {
-                handleTokenRemove(id)
-              }}
-              hideRemoveButton={hideTokenRemoveButtons}
-              size={size}
-              tabIndex={0}
-              {...tokenRest}
-            />
-          ))
-        : null}
     </TextInputWrapper>
   )
 }

diff --git a/src/Token/Token.tsx b/src/Token/Token.tsx
@@ -44,9 +44,23 @@ const DefaultTokenStyled = styled(TokenBase)<TokenProps & {isTokenInteractive: b
   }}
 `
 
-const LeadingVisualContainer = styled('span')`
+const LeadingVisualContainer = styled('span')<Pick<TokenBaseProps, 'size'>>`
   flex-shrink: 0;
   line-height: 0;
+
+  ${props => {
+    switch (props.size) {
+      case 'large':
+      case 'extralarge':
+        return css`
+          margin-right: ${get('space.2')};
+        `
+      default:
+        return css`
+          margin-right: ${get('space.1')};
+        `
+    }
+  }}
 `
 
 const Token = forwardRef<HTMLAnchorElement | HTMLButtonElement | HTMLSpanElement, TokenProps & SxProp>(
@@ -76,7 +90,7 @@ const Token = forwardRef<HTMLAnchorElement | HTMLButtonElement | HTMLSpanElement
         ref={forwardedRef}
       >
         {LeadingVisual ? (
-          <LeadingVisualContainer>
+          <LeadingVisualContainer size={size}>
             <LeadingVisual />
           </LeadingVisualContainer>
         ) : null}

diff --git a/src/Token/TokenBase.tsx b/src/Token/TokenBase.tsx
@@ -48,7 +48,6 @@ const variants = variant<
     fontSize: number
     height: string
     lineHeight: string
-    gap: number
     paddingLeft: number
     paddingRight: number
     paddingTop: number
@@ -60,7 +59,6 @@ const variants = variant<
   variants: {
     small: {
       fontSize: 0,
-      gap: 1,
       height: tokenSizes.small,
       // without setting lineHeight to match height, the "x" appears vertically mis-aligned
       lineHeight: tokenSizes.small,
@@ -73,7 +71,6 @@ const variants = variant<
     },
     medium: {
       fontSize: 0,
-      gap: 1,
       height: tokenSizes.medium,
       lineHeight: tokenSizes.medium,
       paddingLeft: 2,
@@ -83,7 +80,6 @@ const variants = variant<
     },
     large: {
       fontSize: 0,
-      gap: 2,
       height: tokenSizes.large,
       lineHeight: tokenSizes.large,
       paddingLeft: 2,
@@ -93,7 +89,6 @@ const variants = variant<
     },
     extralarge: {
       fontSize: 1,
-      gap: 2,
       height: tokenSizes.extralarge,
       lineHeight: tokenSizes.extralarge,
       paddingLeft: 3,

diff --git a/src/Token/_RemoveTokenButton.tsx b/src/Token/_RemoveTokenButton.tsx
@@ -1,6 +1,6 @@
 import React from 'react'
 import {XIcon} from '@primer/octicons-react'
-import styled from 'styled-components'
+import styled, {css} from 'styled-components'
 import {variant} from 'styled-system'
 import {get} from '../constants'
 import sx, {SxProp} from '../sx'
@@ -50,11 +50,24 @@ const StyledTokenButton = styled.span<TokenButtonProps & SxProp>`
   text-decoration: none;
   padding: 0;
   transform: ${props => `translate(${props.borderOffset}px, -${props.borderOffset}px)`};
-
   align-self: baseline;
   border: 0;
   border-radius: 999px;
 
+  ${props => {
+    switch (props.size) {
+      case 'large':
+      case 'extralarge':
+        return css`
+          margin-left: ${get('space.2')};
+        `
+      default:
+        return css`
+          margin-left: ${get('space.1')};
+        `
+    }
+  }}
+
   &:hover,
   &:focus {
     // TODO: choose a better functional color variable for this