From cb23223471186ff4ae780a541e28c888c8cb6daa Mon Sep 17 00:00:00 2001 From: Reza Rahman Date: Wed, 10 Nov 2021 13:51:31 +0000 Subject: [PATCH 1/5] fix: styling update textinput component with leading icon and block props enabled --- .eslintrc.json | 5 +- src/TextInput.tsx | 5 +- src/stories/TextInput.stories.tsx | 113 ++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 src/stories/TextInput.stories.tsx diff --git a/.eslintrc.json b/.eslintrc.json index 524e372acf2..e07a0902378 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -47,7 +47,10 @@ "jsx-a11y/label-has-for": [ 2, { - "components": [] + "components": [], + "required": { + "some": ["nesting", "id"] + } } ], "camelcase": [ diff --git a/src/TextInput.tsx b/src/TextInput.tsx index 4b30f76fe12..fbb10a34753 100644 --- a/src/TextInput.tsx +++ b/src/TextInput.tsx @@ -37,6 +37,9 @@ const TextInput = React.forwardRef( // this class is necessary to style FilterSearch, plz no touchy! const wrapperClasses = classnames(className, 'TextInput-wrapper') + // temporarily fixes an issue using icon and block in tandem + const customStyles = IconComponent && block && {...sxProp, ...{display: 'flex'}} + return ( ( contrast={contrast} disabled={disabled} hasIcon={!!IconComponent} - sx={sxProp} + sx={customStyles || sxProp} theme={theme} width={widthProp} minWidth={minWidthProp} diff --git a/src/stories/TextInput.stories.tsx b/src/stories/TextInput.stories.tsx new file mode 100644 index 00000000000..0d8c9fb8fdc --- /dev/null +++ b/src/stories/TextInput.stories.tsx @@ -0,0 +1,113 @@ +import React, {useState, ReactNode} from 'react' +import {Meta} from '@storybook/react' + +import {BaseStyles, Box, ThemeProvider, Text} from '..' +import TextInput, {TextInputProps} from '../TextInput' +import {CheckIcon} from '@primer/octicons-react' + +export default { + title: 'Forms/Text Input', + component: TextInput, + decorators: [ + Story => { + return ( + + + {Story()} + + + ) + } + ], + argTypes: { + sx: { + table: { + disable: true + } + }, + block: { + name: 'Block', + defaultValue: false, + control: { + type: 'boolean' + } + }, + disabled: { + name: 'Disabled', + defaultValue: false, + control: { + type: 'boolean' + } + }, + variant: { + name: 'Variants', + options: ['small', 'medium', 'large'], + control: {type: 'radio'} + } + } +} as Meta + +const Label = ({htmlFor, children}: {htmlFor: string; children: ReactNode}) => ( + + {children} + +) + +export const Default = (args: TextInputProps) => { + const [value, setValue] = useState('') + + const handleChange = (event: React.ChangeEvent) => { + setValue(event.target.value) + } + + const inputId = 'basic-text-input' + + return ( +
+
+
+ +
+
+ +
+
+
+ ) +} + +export const WithLeadingIcon = (args: TextInputProps) => { + const [value, setValue] = useState('') + + const handleChange = (event: React.ChangeEvent) => { + setValue(event.target.value) + } + + const inputId = 'basic-text-input-with-leading-icon' + + return ( +
+ +
+ + + ) +} + +export const Password = (args: TextInputProps) => { + const [value, setValue] = useState('') + + const handleChange = (event: React.ChangeEvent) => { + setValue(event.target.value) + } + + const inputId = 'basic-text-input-as-password' + + return ( +
+ +
+ + + ) +} From a18c01e58ea67e51e62ba7bb2d516f23104992c3 Mon Sep 17 00:00:00 2001 From: Reza Rahman Date: Wed, 10 Nov 2021 13:53:39 +0000 Subject: [PATCH 2/5] add changeset --- .changeset/hip-ravens-sell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/hip-ravens-sell.md diff --git a/.changeset/hip-ravens-sell.md b/.changeset/hip-ravens-sell.md new file mode 100644 index 00000000000..b5ff9010577 --- /dev/null +++ b/.changeset/hip-ravens-sell.md @@ -0,0 +1,5 @@ +--- +'@primer/components': patch +--- + +Fixes a styling bug in TextInput components while using its `icon` and `block` props together From 5f44ebb26b19fcd4c21bf2c8b0c14b44b0e87bce Mon Sep 17 00:00:00 2001 From: Reza Rahman Date: Wed, 10 Nov 2021 14:23:28 +0000 Subject: [PATCH 3/5] revert eslint rule change --- .eslintrc.json | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index e07a0902378..8e413d90680 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -44,15 +44,20 @@ "react/prop-types": 0, "react/display-name": 0, "react-hooks/exhaustive-deps": "error", - "jsx-a11y/label-has-for": [ - 2, + "jsx-a11y/label-has-associated-control": [ + "error", { - "components": [], "required": { "some": ["nesting", "id"] } } ], + "jsx-a11y/label-has-for": [ + 2, + { + "components": [] + } + ], "camelcase": [ "error", { From a511b2df9af604d4a769978d3692d854101d13eb Mon Sep 17 00:00:00 2001 From: Reza Rahman Date: Wed, 10 Nov 2021 14:27:19 +0000 Subject: [PATCH 4/5] reverting eslint rule again --- .eslintrc.json | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 8e413d90680..524e372acf2 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -44,14 +44,6 @@ "react/prop-types": 0, "react/display-name": 0, "react-hooks/exhaustive-deps": "error", - "jsx-a11y/label-has-associated-control": [ - "error", - { - "required": { - "some": ["nesting", "id"] - } - } - ], "jsx-a11y/label-has-for": [ 2, { From 2c6b139dd919e7ca7d97e993372938f3ab00952b Mon Sep 17 00:00:00 2001 From: Reza Rahman Date: Wed, 10 Nov 2021 18:28:28 +0000 Subject: [PATCH 5/5] refactor with simpler solution --- src/TextInput.tsx | 5 +---- src/_TextInputWrapper.tsx | 7 +++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/TextInput.tsx b/src/TextInput.tsx index fbb10a34753..4b30f76fe12 100644 --- a/src/TextInput.tsx +++ b/src/TextInput.tsx @@ -37,9 +37,6 @@ const TextInput = React.forwardRef( // this class is necessary to style FilterSearch, plz no touchy! const wrapperClasses = classnames(className, 'TextInput-wrapper') - // temporarily fixes an issue using icon and block in tandem - const customStyles = IconComponent && block && {...sxProp, ...{display: 'flex'}} - return ( ( contrast={contrast} disabled={disabled} hasIcon={!!IconComponent} - sx={customStyles || sxProp} + sx={sxProp} theme={theme} width={widthProp} minWidth={minWidthProp} diff --git a/src/_TextInputWrapper.tsx b/src/_TextInputWrapper.tsx index 83822045cb5..fbc4da61e13 100644 --- a/src/_TextInputWrapper.tsx +++ b/src/_TextInputWrapper.tsx @@ -91,6 +91,13 @@ const TextInputWrapper = styled.span` display: block; width: 100%; `} + + ${props => + props.block && + props.hasIcon && + css` + display: flex; + `} // Ensures inputs don't zoom on mobile but are body-font size on desktop @media (min-width: ${get('breakpoints.1')}) {