From fdfb6d759aa4d4da704aa50a15fc927d0fd6a7d9 Mon Sep 17 00:00:00 2001 From: Lee White Date: Tue, 9 Aug 2022 14:23:54 -0700 Subject: [PATCH] fix(form-pill-group): small style bugs (#2589) * fix(form-pill-group): style bugs * fix: final hover styles fix * chore: update yarn.lock --- .changeset/fair-grapes-raise.md | 6 +++ .../components/form-pill-group/package.json | 3 ++ .../form-pill-group/src/FormPill.styles.ts | 52 ++++++++++++++++--- .../form-pill-group/src/FormPill.tsx | 24 +++++++-- .../form-pill-group/src/FormPillButton.tsx | 4 +- .../form-pill-group/src/PillCloseIcon.tsx | 22 +++++++- yarn.lock | 1 + 7 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 .changeset/fair-grapes-raise.md diff --git a/.changeset/fair-grapes-raise.md b/.changeset/fair-grapes-raise.md new file mode 100644 index 0000000000..adde8b97a8 --- /dev/null +++ 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 index 94848469fd..3c13da04ba 100644 --- 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 index 5eb3cd7082..6a0210c595 100644 --- 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,16 +129,14 @@ 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', @@ -143,7 +144,7 @@ export const closeStyles: VariantStyles = { }, }; -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 index 1ad2370175..a7536fbfcd 100644 --- 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 { selected?: boolean; @@ -63,7 +63,11 @@ export const FormPill = React.forwardRef( 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( [onDismiss, next] ); return ( - + ( {props.children} {isDismissable && !disabled ? ( - + ) : null} ); diff --git a/packages/paste-core/components/form-pill-group/src/FormPillButton.tsx b/packages/paste-core/components/form-pill-group/src/FormPillButton.tsx index 6a698712e6..fe31ab616d 100644 --- 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 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 {variant === 'error' ? ( <> - + {i18nErrorLabel} ) : 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 index c4e50138c1..6e33148d4c 100644 --- a/packages/paste-core/components/form-pill-group/src/PillCloseIcon.tsx +++ b/packages/paste-core/components/form-pill-group/src/PillCloseIcon.tsx @@ -2,14 +2,24 @@ 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 = ({ @@ -17,8 +27,16 @@ export const PillCloseIcon: React.FC = ({ 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 (