From 0dd7d4af878dc8c3af117f8cea542de6f82c9917 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Fri, 15 Apr 2022 17:57:13 -0400 Subject: [PATCH 01/10] makes hover background appear inset --- src/_TextInputInnerAction.tsx | 29 ++++++- .../__snapshots__/TextInput.test.tsx.snap | 76 +++++++++++++++---- 2 files changed, 89 insertions(+), 16 deletions(-) diff --git a/src/_TextInputInnerAction.tsx b/src/_TextInputInnerAction.tsx index a9382b0cf2e..955b5796b48 100644 --- a/src/_TextInputInnerAction.tsx +++ b/src/_TextInputInnerAction.tsx @@ -16,7 +16,29 @@ type TextInputActionProps = Omit, 'aria-label } & SxProp const invisibleButtonStyleOverrides = { - color: 'fg.default' + color: 'fg.default', + margin: 1, + paddingTop: '2px', + paddingRight: '4px', + paddingBottom: '2px', + paddingLeft: '4px', + position: 'relative', + + '@media (pointer: fine)': { + ':after': { + content: '""', + position: 'absolute', + left: 0, + right: 0, + transform: 'translateY(-50%)', + top: '50%', + minHeight: '44px' + } + } +} + +const solidButtonStyleOverrides = { + margin: 1 } const ConditionalTooltip: React.FC<{ @@ -44,7 +66,9 @@ const ConditionalTooltip: React.FC<{ const TextInputAction = forwardRef( ({'aria-label': ariaLabel, children, icon, sx: sxProp, variant, ...rest}, forwardedRef) => { const sx = - variant === 'invisible' ? merge(invisibleButtonStyleOverrides, sxProp || {}) : sxProp + variant === 'invisible' + ? merge(invisibleButtonStyleOverrides, sxProp || {}) + : merge(solidButtonStyleOverrides, sxProp || {}) if ((icon && !ariaLabel) || (!children && !ariaLabel)) { // eslint-disable-next-line no-console @@ -60,6 +84,7 @@ const TextInputAction = forwardRef( type="button" icon={icon} aria-label={ariaLabel} + size="small" sx={sx} {...rest} ref={forwardedRef} diff --git a/src/__tests__/__snapshots__/TextInput.test.tsx.snap b/src/__tests__/__snapshots__/TextInput.test.tsx.snap index f8408bbeaac..5479fef11c1 100644 --- a/src/__tests__/__snapshots__/TextInput.test.tsx.snap +++ b/src/__tests__/__snapshots__/TextInput.test.tsx.snap @@ -1179,14 +1179,16 @@ exports[`TextInput renders trailingAction icon button 1`] = ` -webkit-text-decoration: none; text-decoration: none; text-align: center; - padding-top: 6px; - padding-bottom: 6px; - padding-left: 8px; - padding-right: 8px; - font-size: 14px; + padding-top: 2px; + padding-bottom: 2px; + padding-left: 4px; + padding-right: 4px; + font-size: 12px; color: #24292f; background-color: transparent; box-shadow: none; + margin: 4px; + position: relative; } .c3:disabled { @@ -1203,7 +1205,7 @@ exports[`TextInput renders trailingAction icon button 1`] = ` } .c3 [data-component=ButtonCounter] { - font-size: 14px; + font-size: 12px; } .c3:hover:not([disabled]) { @@ -1540,6 +1542,20 @@ exports[`TextInput renders trailingAction icon button 1`] = ` } } +@media (pointer:fine) { + .c3:after { + content: ""; + position: absolute; + left: 0; + right: 0; + -webkit-transform: translateY(-50%); + -ms-transform: translateY(-50%); + transform: translateY(-50%); + top: 50%; + min-height: 44px; + } +} + @media (min-width:768px) { .c0 { font-size: 14px; @@ -1630,14 +1646,16 @@ exports[`TextInput renders trailingAction text button 1`] = ` text-align: center; display: grid; grid-template-areas: "leadingIcon text trailingIcon"; - padding-top: 4px; - padding-bottom: 4px; - padding-left: 12px; - padding-right: 12px; + padding-top: 2px; + padding-bottom: 2px; + padding-left: 4px; + padding-right: 4px; font-size: 12px; color: #24292f; background-color: transparent; box-shadow: none; + margin: 4px; + position: relative; } .c2:disabled { @@ -1788,6 +1806,20 @@ exports[`TextInput renders trailingAction text button 1`] = ` } } +@media (pointer:fine) { + .c2:after { + content: ""; + position: absolute; + left: 0; + right: 0; + -webkit-transform: translateY(-50%); + -ms-transform: translateY(-50%); + transform: translateY(-50%); + top: 50%; + min-height: 44px; + } +} + @media (min-width:768px) { .c0 { font-size: 14px; @@ -1851,14 +1883,16 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = ` text-align: center; display: grid; grid-template-areas: "leadingIcon text trailingIcon"; - padding-top: 4px; - padding-bottom: 4px; - padding-left: 12px; - padding-right: 12px; + padding-top: 2px; + padding-bottom: 2px; + padding-left: 4px; + padding-right: 4px; font-size: 12px; color: #24292f; background-color: transparent; box-shadow: none; + margin: 4px; + position: relative; } .c3:disabled { @@ -2229,6 +2263,20 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = ` } } +@media (pointer:fine) { + .c3:after { + content: ""; + position: absolute; + left: 0; + right: 0; + -webkit-transform: translateY(-50%); + -ms-transform: translateY(-50%); + transform: translateY(-50%); + top: 50%; + min-height: 44px; + } +} + @media (min-width:768px) { .c0 { font-size: 14px; From c4a48b2952a2c19ca792ae4549b5cdd6f49b0de5 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Fri, 15 Apr 2022 18:07:29 -0400 Subject: [PATCH 02/10] deprecates children prop - only icons should be used for text input inner actions --- docs/content/TextInput.mdx | 46 +++++++++----------------- src/_TextInputInnerAction.tsx | 2 ++ src/stories/TextInput.stories.tsx | 55 ++++++++++--------------------- 3 files changed, 35 insertions(+), 68 deletions(-) diff --git a/docs/content/TextInput.mdx b/docs/content/TextInput.mdx index 4ec34745bc8..ded48078a88 100644 --- a/docs/content/TextInput.mdx +++ b/docs/content/TextInput.mdx @@ -111,37 +111,21 @@ render() ### With trailing action ```jsx live - - - Icon action - { - alert('clear input') - }} - icon={XIcon} - aria-label="Clear input" - sx={{color: 'fg.subtle'}} - /> - } - /> - - - Text action - { - alert('clear input') - }} - > - Clear - - } - /> - - + + Icon action + { + alert('clear input') + }} + icon={XIcon} + aria-label="Clear input" + sx={{color: 'fg.subtle'}} + /> + } + /> + ``` ### With error and warning states diff --git a/src/_TextInputInnerAction.tsx b/src/_TextInputInnerAction.tsx index 955b5796b48..b6ff23767ba 100644 --- a/src/_TextInputInnerAction.tsx +++ b/src/_TextInputInnerAction.tsx @@ -5,6 +5,8 @@ import {ButtonProps} from './Button' import {BetterSystemStyleObject, merge, SxProp} from './sx' type TextInputActionProps = Omit, 'aria-label' | 'size'> & { + /** @deprecated Text input action buttons will only use icon buttons */ + children?: React.ReactNode /** Text that appears in a tooltip. If an icon is passed, this is also used as the label used by assistive technologies. */ ['aria-label']?: string /** The icon to render inside the button */ diff --git a/src/stories/TextInput.stories.tsx b/src/stories/TextInput.stories.tsx index 741447fdfc3..f25182f7cf7 100644 --- a/src/stories/TextInput.stories.tsx +++ b/src/stories/TextInput.stories.tsx @@ -149,43 +149,24 @@ export const WithTrailingAction = (args: TextInputProps) => { return (
- - - Icon action - { - setValue('') - }} - icon={XCircleFillIcon} - aria-label="Clear input" - sx={{color: 'fg.subtle'}} - /> - } - value={value} - onChange={handleChange} - {...args} - /> - - - Text action - { - setValue('') - }} - > - Clear - - } - value={value} - onChange={handleChange} - {...args} - /> - - + + Icon action + { + setValue('') + }} + icon={XCircleFillIcon} + aria-label="Clear input" + sx={{color: 'fg.subtle'}} + /> + } + value={value} + onChange={handleChange} + {...args} + /> +
) } From 75ddbbf6cb313edb4a481c047f09175ae63e4183 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 19 Apr 2022 09:08:49 -0400 Subject: [PATCH 03/10] deprecates 'variant' prop for TextInputInnerAction --- docs/content/TextInput.mdx | 3 ++- src/_TextInputInnerAction.tsx | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/content/TextInput.mdx b/docs/content/TextInput.mdx index ded48078a88..69ba4872659 100644 --- a/docs/content/TextInput.mdx +++ b/docs/content/TextInput.mdx @@ -299,7 +299,8 @@ render() , 'aria-label' | 'size'> & { - /** @deprecated Text input action buttons will only use icon buttons */ + /** @deprecated Text input action buttons should only use icon buttons */ children?: React.ReactNode /** Text that appears in a tooltip. If an icon is passed, this is also used as the label used by assistive technologies. */ ['aria-label']?: string /** The icon to render inside the button */ icon?: React.FunctionComponent /** + * @deprecated Text input action buttons should only use the 'invisible' button variant * Determine's the styles on a button one of 'default' | 'primary' | 'invisible' | 'danger' */ variant?: ButtonProps['variant'] From 5c9dba713a4411b6aa6df16bf4afdaad4cc30d11 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 19 Apr 2022 09:41:26 -0400 Subject: [PATCH 04/10] limits larger inner action tap target for coarse pointers --- src/_TextInputInnerAction.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_TextInputInnerAction.tsx b/src/_TextInputInnerAction.tsx index bb148ba4c6c..a3dc9c61007 100644 --- a/src/_TextInputInnerAction.tsx +++ b/src/_TextInputInnerAction.tsx @@ -27,7 +27,7 @@ const invisibleButtonStyleOverrides = { paddingLeft: '4px', position: 'relative', - '@media (pointer: fine)': { + '@media (pointer: coarse)': { ':after': { content: '""', position: 'absolute', From aa1020e4662e9927e99797ebb41b5ed66c854e9d Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 19 Apr 2022 09:49:35 -0400 Subject: [PATCH 05/10] adds changeset --- .changeset/tall-dancers-exercise.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/tall-dancers-exercise.md diff --git a/.changeset/tall-dancers-exercise.md b/.changeset/tall-dancers-exercise.md new file mode 100644 index 00000000000..309259b6596 --- /dev/null +++ b/.changeset/tall-dancers-exercise.md @@ -0,0 +1,8 @@ +--- +'@primer/react': patch +--- + +- Inner action's hover bg should not touch the text input edges +- Increases touch target area of the inner action button +- Deprecation: we only want to allow inner action buttons to be icon buttons +- Deprecation: we only want to allow inner action buttons to be the 'invisible' variant From f4aded1fcdce8a96d02d2909a826feb091a16497 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 19 Apr 2022 11:51:53 -0400 Subject: [PATCH 06/10] updates snapshots --- src/__tests__/__snapshots__/TextInput.test.tsx.snap | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/__tests__/__snapshots__/TextInput.test.tsx.snap b/src/__tests__/__snapshots__/TextInput.test.tsx.snap index 5479fef11c1..0723b8557d2 100644 --- a/src/__tests__/__snapshots__/TextInput.test.tsx.snap +++ b/src/__tests__/__snapshots__/TextInput.test.tsx.snap @@ -1542,7 +1542,7 @@ exports[`TextInput renders trailingAction icon button 1`] = ` } } -@media (pointer:fine) { +@media (pointer:coarse) { .c3:after { content: ""; position: absolute; @@ -1806,7 +1806,7 @@ exports[`TextInput renders trailingAction text button 1`] = ` } } -@media (pointer:fine) { +@media (pointer:coarse) { .c2:after { content: ""; position: absolute; @@ -2263,7 +2263,7 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = ` } } -@media (pointer:fine) { +@media (pointer:coarse) { .c3:after { content: ""; position: absolute; From f2bff9ecb057b8b077cb25efa30066f09fd118f3 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 20 Apr 2022 07:49:45 -0400 Subject: [PATCH 07/10] prevents tooltip from appearing until button is actually hovered --- src/_TextInputInnerAction.tsx | 9 +- .../__snapshots__/TextInput.test.tsx.snap | 323 +++++++++--------- 2 files changed, 168 insertions(+), 164 deletions(-) diff --git a/src/_TextInputInnerAction.tsx b/src/_TextInputInnerAction.tsx index a3dc9c61007..255f8c842f6 100644 --- a/src/_TextInputInnerAction.tsx +++ b/src/_TextInputInnerAction.tsx @@ -20,7 +20,6 @@ type TextInputActionProps = Omit, 'aria-label const invisibleButtonStyleOverrides = { color: 'fg.default', - margin: 1, paddingTop: '2px', paddingRight: '4px', paddingBottom: '2px', @@ -40,10 +39,6 @@ const invisibleButtonStyleOverrides = { } } -const solidButtonStyleOverrides = { - margin: 1 -} - const ConditionalTooltip: React.FC<{ ['aria-label']?: string children: React.ReactNode @@ -71,7 +66,7 @@ const TextInputAction = forwardRef( const sx = variant === 'invisible' ? merge(invisibleButtonStyleOverrides, sxProp || {}) - : merge(solidButtonStyleOverrides, sxProp || {}) + : sxProp || {} if ((icon && !ariaLabel) || (!children && !ariaLabel)) { // eslint-disable-next-line no-console @@ -79,7 +74,7 @@ const TextInputAction = forwardRef( } return ( - + {icon && !children ? (