fix(form-pill-group): small style bugs (#2589)

* fix(form-pill-group): style bugs * fix: final hover styles fix * chore: update yarn.lock
twilio-labs · Aug 9, 2022 · fdfb6d7 · fdfb6d7
1 parent e21a9ff
commit fdfb6d7
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 14 deletions.
diff --git a/.changeset/fair-grapes-raise.md b/.changeset/fair-grapes-raise.md
@@ -0,0 +1,6 @@
+---
+'@twilio-paste/form-pill-group': patch
+'@twilio-paste/core': patch
+---
+
+[Form Pill Group] fix small style issues
diff --git a/packages/paste-core/components/form-pill-group/package.json b/packages/paste-core/components/form-pill-group/package.json
@@ -24,6 +24,9 @@
     "clean": "rm -rf ./dist",
     "tsc": "tsc"
   },
+  "dependencies": {
+    "deepmerge": "4.2.2"
+  },
   "peerDependencies": {
     "@twilio-paste/animation-library": "^0.3.2",
     "@twilio-paste/box": "^6.0.0",

diff --git a/packages/paste-core/components/form-pill-group/src/FormPill.styles.ts b/packages/paste-core/components/form-pill-group/src/FormPill.styles.ts
@@ -1,7 +1,10 @@
+import type {BoxStyleProps} from '@twilio-paste/box';
 import type {VariantStyles} from './types';
 
 /**
  * Wrapper styles
+ * If the pill is hoverable (i.e. can be selected), we set the color of the 'x' to inherit and set the color on the wrapper. This is so that when anywhere on the pill is hovered, the 'x' matches the rest of the content.
+ * If the pill is not hoverable, we set the color of the 'x' to the correct tokens.
  */
 
 export const wrapperStyles: VariantStyles = {
@@ -97,7 +100,7 @@ export const hoverPillStyles: VariantStyles = {
     _selected_hover: {
       backgroundColor: 'colorBackgroundPrimary',
       borderColor: 'transparent',
-      color: 'inherit',
+      color: 'colorTextInverse',
     },
     _focus_hover: {
       borderColor: 'transparent',
@@ -109,12 +112,12 @@ export const hoverPillStyles: VariantStyles = {
 
     _hover: {
       borderColor: 'colorBorderErrorStronger',
-      color: 'inherit',
+      color: 'colorTextErrorStronger',
     },
     _selected_hover: {
       backgroundColor: 'colorBackgroundErrorStrongest',
       borderColor: 'transparent',
-      color: 'inherit',
+      color: 'colorTextWeakest',
     },
     _focus_hover: {
       borderColor: 'transparent',
@@ -126,24 +129,22 @@ export const hoverPillStyles: VariantStyles = {
  * Close icon styles
  */
 
-export const closeStyles: VariantStyles = {
+export const baseCloseStyles: VariantStyles = {
   default: {
-    color: 'inherit',
     _hover: {
       cursor: 'pointer',
       borderColor: 'colorBorderPrimaryStronger',
     },
   },
   error: {
-    color: 'inherit',
     _hover: {
       cursor: 'pointer',
       borderColor: 'colorBorderErrorStronger',
     },
   },
 };
 
-export const selectedCloseStyles: VariantStyles = {
+export const selectedBaseCloseStyles: VariantStyles = {
   default: {
     _hover: {
       cursor: 'pointer',
@@ -159,3 +160,40 @@ export const selectedCloseStyles: VariantStyles = {
     },
   },
 };
+
+export const closeInheritColorStyles: BoxStyleProps = {
+  color: 'inherit',
+  _hover: {
+    color: 'inherit',
+  },
+};
+
+export const closeColorStyles: VariantStyles = {
+  default: {
+    color: 'colorTextIcon',
+    _hover: {
+      color: 'colorTextLinkStronger',
+    },
+  },
+  error: {
+    color: 'colorTextIcon',
+    _hover: {
+      color: 'colorTextErrorStronger',
+    },
+  },
+};
+
+export const selectedCloseColorStyles: VariantStyles = {
+  default: {
+    color: 'colorTextWeakest',
+    _hover: {
+      color: 'colorTextInverse',
+    },
+  },
+  error: {
+    color: 'colorTextInverse',
+    _hover: {
+      color: 'colorTextWeakest',
+    },
+  },
+};
diff --git a/packages/paste-core/components/form-pill-group/src/FormPill.tsx b/packages/paste-core/components/form-pill-group/src/FormPill.tsx
@@ -5,8 +5,8 @@ import {CompositeItem} from '@twilio-paste/reakit-library';
 import type {CompositeStateReturn} from '@twilio-paste/reakit-library';
 import {PillCloseIcon} from './PillCloseIcon';
 import {FormPillButton} from './FormPillButton';
-import {wrapperStyles, selectedWrapperStyles} from './FormPill.styles';
 import type {PillVariant} from './types';
+import {selectedWrapperStyles, wrapperStyles} from './FormPill.styles';
 
 interface FormPillProps extends CompositeStateReturn, Pick<BoxProps, 'element'> {
   selected?: boolean;
@@ -63,7 +63,11 @@ export const FormPill = React.forwardRef<HTMLElement, FormPillProps>(
     const isHoverable = onSelect != null;
     const isDismissable = onDismiss != null;
 
-    const computedStyles = selected ? selectedWrapperStyles[variant] : wrapperStyles[variant];
+    let computedStyles = {};
+
+    if (isHoverable) {
+      computedStyles = selected ? selectedWrapperStyles[variant] : wrapperStyles[variant];
+    }
 
     // Handles delete and backspace keypresses
     const handleKeydown = React.useCallback(
@@ -80,7 +84,13 @@ export const FormPill = React.forwardRef<HTMLElement, FormPillProps>(
       [onDismiss, next]
     );
     return (
-      <Box element={`${element}_WRAPPER`} position="relative" display="inline-block" {...computedStyles}>
+      <Box
+        element={`${element}_WRAPPER`}
+        position="relative"
+        display="inline-block"
+        borderRadius="borderRadiusPill"
+        {...computedStyles}
+      >
         <CompositeItem
           {...safelySpreadBoxProps(props)}
           element={element}
@@ -100,7 +110,13 @@ export const FormPill = React.forwardRef<HTMLElement, FormPillProps>(
           {props.children}
         </CompositeItem>
         {isDismissable && !disabled ? (
-          <PillCloseIcon element={`${element}_CLOSE`} onClick={onDismiss} selected={selected} variant={variant} />
+          <PillCloseIcon
+            element={`${element}_CLOSE`}
+            onClick={onDismiss}
+            selected={selected}
+            variant={variant}
+            pillIsHoverable={isHoverable}
+          />
         ) : null}
       </Box>
     );

diff --git a/packages/paste-core/components/form-pill-group/src/FormPillButton.tsx b/packages/paste-core/components/form-pill-group/src/FormPillButton.tsx
@@ -49,10 +49,12 @@ export const FormPillButton = React.forwardRef<HTMLElement, FormPillStylesProps>
         role="option"
         type="button"
         as="button"
+        margin="space0"
         position="relative"
         borderRadius="borderRadiusPill"
         borderWidth="borderWidth10"
         borderColor="transparent"
+        borderStyle="solid"
         cursor="default"
         height="sizeIcon40"
         fontFamily="inherit"
@@ -67,7 +69,7 @@ export const FormPillButton = React.forwardRef<HTMLElement, FormPillStylesProps>
         <Box display="flex" alignItems="center" columnGap="space20" opacity={isDisabled ? 0.3 : 1}>
           {variant === 'error' ? (
             <>
-              <ErrorIcon decorative />
+              <ErrorIcon decorative size="sizeIcon10" />
               <ScreenReaderOnly>{i18nErrorLabel}</ScreenReaderOnly>
             </>
           ) : null}

diff --git a/packages/paste-core/components/form-pill-group/src/PillCloseIcon.tsx b/packages/paste-core/components/form-pill-group/src/PillCloseIcon.tsx
@@ -2,23 +2,41 @@ import * as React from 'react';
 import {Box} from '@twilio-paste/box';
 import type {BoxProps} from '@twilio-paste/box';
 import {CloseIcon} from '@twilio-paste/icons/esm/CloseIcon';
-import {selectedCloseStyles, closeStyles} from './FormPill.styles';
+import {
+  baseCloseStyles,
+  closeColorStyles,
+  closeInheritColorStyles,
+  selectedBaseCloseStyles,
+  selectedCloseColorStyles,
+} from './FormPill.styles';
 import type {PillVariant} from './types';
 
+// This module can only be referenced with ECMAScript imports/exports by turning on the 'esModuleInterop' flag and referencing its default export
+const merge = require('deepmerge');
+
 interface PillCloseIconProps {
   onClick?: () => void;
   selected?: boolean;
   variant?: PillVariant;
   element?: BoxProps['element'];
+  pillIsHoverable?: boolean;
 }
 
 export const PillCloseIcon: React.FC<PillCloseIconProps> = ({
   element = 'FORM_PILL_CLOSE',
   onClick = () => {},
   selected = false,
   variant = 'default',
+  pillIsHoverable = false,
 }) => {
-  const computedStyles = selected ? selectedCloseStyles[variant] : closeStyles[variant];
+  const baseStyles = selected ? selectedBaseCloseStyles[variant] : baseCloseStyles[variant];
+  let colorStyles = selected ? selectedCloseColorStyles[variant] : closeColorStyles[variant];
+
+  if (pillIsHoverable) {
+    colorStyles = closeInheritColorStyles;
+  }
+
+  const computedStyles = merge(baseStyles, colorStyles);
 
   return (
     <Box

diff --git a/yarn.lock b/yarn.lock
@@ -10540,6 +10540,7 @@ __metadata:
     "@twilio-paste/theme": ^7.0.1
     "@twilio-paste/types": ^3.1.8
     "@twilio-paste/uid-library": ^0.2.5
+    deepmerge: 4.2.2
     prop-types: ^15.7.2
     react: ^17.0.2
     react-dom: ^17.0.2