Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gold-dingos-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': major
---

Removes styled-system usage from TextInputWrapper and related components and Storybook stories.
36 changes: 0 additions & 36 deletions e2e/components/TextInput.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,24 +219,6 @@ test.describe('TextInput', () => {
}
})

test.describe('Dev: With Sx', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-textinput-dev--with-sx',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`TextInput.Dev.WithSx.${theme}.png`)
})
})
}
})

test.describe('Dev: With CSS', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand All @@ -254,22 +236,4 @@ test.describe('TextInput', () => {
})
}
})

test.describe('Dev: With Sx and CSS', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-textinput-dev--with-sx-and-css',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`TextInput.Dev.WithSxAndCSS.${theme}.png`)
})
})
}
})
})
12 changes: 12 additions & 0 deletions packages/react/src/Overlay/Overlay.features.stories.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
.MemexOverlayIssueTitleInput {
padding-block: var(--base-size-2);
padding-inline: var(--base-size-8);
text-align: left;
color: var(--fgColor-default);
}

.MemexOverlayIssueTitleInput input {
font-weight: var(--base-text-weight-semibold);
font-size: var(--text-title-size-medium);
padding-inline: 0;
}
23 changes: 10 additions & 13 deletions packages/react/src/Overlay/Overlay.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ import {
ActionList,
ActionMenu,
useFocusTrap,
Stack,
Textarea,
} from '..'
import {Tooltip} from '../TooltipV2'
import classes from './Overlay.features.stories.module.css'

export default {
title: 'Private/Components/Overlay/Features',
Expand Down Expand Up @@ -387,17 +390,17 @@ export const NestedOverlays = ({role, open}: Args) => {
aria-label={role === 'dialog' ? 'Create a list' : undefined}
ref={secondaryContainer}
>
<Box as="form" sx={{display: 'flex', flexDirection: 'column', p: 3}} aria-label="Set Duration Form">
<Text color="fg.muted" sx={{fontSize: 1, mb: 3}}>
<Stack as="form" gap="condensed" padding="normal" aria-label="Set Duration Form">
<Text color="fg.muted" size="medium">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you convert the color here?

Create a list to organize your starred repositories.
</Text>
<TextInput placeholder="Name this list" sx={{mb: 2}} />
<TextInput as="textarea" placeholder="Write a description" rows={3} sx={{mb: 2, textarea: {p: 2}}} />
<TextInput placeholder="Name this list" />
<Textarea placeholder="Write a description" rows={3} />

<Button variant="primary" onClick={() => setCreateListOverlayOpen(!createListOverlayOpen)}>
Create
</Button>
</Box>
</Stack>
</Overlay>
)}
</Overlay>
Expand Down Expand Up @@ -486,14 +489,8 @@ export const MemexIssueOverlay = ({role, open}: Args) => {
}
}}
ref={inputRef}
sx={{
width: '100%',
py: '2px',
px: '7px',
textAlign: 'left',
color: 'fg.default',
input: {fontWeight: 'bold', fontSize: 4, px: 0},
}}
block
className={classes.MemexOverlayIssueTitleInput}
/>
) : (
<Button
Expand Down
2 changes: 0 additions & 2 deletions packages/react/src/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ const Select = React.forwardRef<HTMLSelectElement, SelectProps>(
size,
required,
validationStatus,
sx,
...rest
}: SelectProps,
ref,
Expand All @@ -58,7 +57,6 @@ const Select = React.forwardRef<HTMLSelectElement, SelectProps>(
size={size}
validationStatus={validationStatus}
className={clsx(classes.TextInputWrapper, className)}
sx={sx}
>
<select
{...rest}
Expand Down
5 changes: 5 additions & 0 deletions packages/react/src/SelectPanel/SelectPanel.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@
color: var(--fgColor-muted);
}

.FilterInputWrapper {
margin: var(--base-size-8);
width: auto;
}

.Notice {
display: flex;
padding-top: var(--base-size-12);
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/SelectPanel/SelectPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -656,10 +656,10 @@ function Panel({

const extendedTextInputProps: Partial<TextInputProps> = useMemo(() => {
return {
sx: {m: 2},
contrast: true,
leadingVisual: SearchIcon,
'aria-label': inputLabel,
className: classes.FilterInputWrapper,
...textInputProps,
}
}, [inputLabel, textInputProps])
Expand Down
24 changes: 3 additions & 21 deletions packages/react/src/TextInput/TextInput.dev.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type React from 'react'
import type {Meta} from '@storybook/react-vite'
import {Box, FormControl} from '..'
import {FormControl} from '..'
import TextInput from '.'
import {textInputExcludedControlKeys} from '../utils/story-helpers'

Expand All @@ -11,28 +11,10 @@ export default {
} as Meta<React.ComponentProps<typeof TextInput>>

export const WithCSS = () => (
<Box as="form">
<form>
<FormControl>
<FormControl.Label>Default label</FormControl.Label>
<TextInput className="testCustomClassnameBorderColor" />
</FormControl>
</Box>
)

export const WithSx = () => (
<Box as="form">
<FormControl>
<FormControl.Label>Default label</FormControl.Label>
<TextInput sx={{borderColor: 'red'}} />
</FormControl>
</Box>
)

export const WithSxAndCSS = () => (
<Box as="form">
<FormControl>
<FormControl.Label>Default label</FormControl.Label>
<TextInput sx={{borderColor: 'red'}} className="testCustomClassnameBorderColor" />
</FormControl>
</Box>
</form>
)
23 changes: 1 addition & 22 deletions packages/react/src/TextInput/TextInput.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -140,27 +140,6 @@
"deprecated": true,
"description": "(Use size) Creates a smaller or larger input than the default."
},
{
"name": "width",
"type": "string | number | Array<string | number>",
"defaultValue": "",
"deprecated": true,
"description": "(Use sx prop) Set the width of the input"
},
{
"name": "maxWidth",
"type": "string | number | Array<string | number>",
"defaultValue": "",
"deprecated": true,
"description": "(Use sx prop) Set the maximum width of the input"
},
{
"name": "minWidth",
"type": "string | number | Array<string | number>",
"defaultValue": "",
"deprecated": true,
"description": "(Use sx prop) Set the minimum width of the input"
},
{
"name": "icon",
"type": "React.ComponentType",
Expand Down Expand Up @@ -228,4 +207,4 @@
}
}
]
}
}
23 changes: 1 addition & 22 deletions packages/react/src/TextInput/TextInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,7 @@ export type TextInputNonPassthroughProps = {
*/
trailingAction?: React.ReactElement<React.HTMLProps<HTMLButtonElement>>
} & Partial<
Pick<
StyledWrapperProps,
| 'block'
| 'contrast'
| 'disabled'
| 'monospace'
| 'sx'
| 'width'
| 'maxWidth'
| 'minWidth'
| 'variant'
| 'size'
| 'validationStatus'
>
Pick<StyledWrapperProps, 'block' | 'contrast' | 'disabled' | 'monospace' | 'variant' | 'size' | 'validationStatus'>
>

export type TextInputProps = Merge<React.ComponentPropsWithoutRef<'input'>, TextInputNonPassthroughProps>
Expand All @@ -75,14 +62,10 @@ const TextInput = React.forwardRef<HTMLInputElement, TextInputProps>(
loaderText = 'Loading',
monospace,
validationStatus,
sx: sxProp,
size: sizeProp,
onFocus,
onBlur,
// start deprecated props
width: widthProp,
minWidth: minWidthProp,
maxWidth: maxWidthProp,
variant: variantProp,
// end deprecated props
type = 'text',
Expand Down Expand Up @@ -137,11 +120,7 @@ const TextInput = React.forwardRef<HTMLInputElement, TextInputProps>(
contrast={contrast}
disabled={disabled}
monospace={monospace}
sx={sxProp}
size={sizeProp}
width={widthProp}
minWidth={minWidthProp}
maxWidth={maxWidthProp}
variant={variantProp}
hasLeadingVisual={Boolean(LeadingVisual || showLeadingLoadingIndicator)}
hasTrailingVisual={Boolean(TrailingVisual || showTrailingLoadingIndicator)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,13 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
className,
block,
disabled,
sx: sxProp,
tokens,
onTokenRemove,
tokenComponent: TokenComponent = Token,
preventTokenWrapping = false,
size = 'xlarge',
hideTokenRemoveButtons = false,
maxHeight,
width: widthProp,
minWidth: minWidthProp,
maxWidth: maxWidthProp,
validationStatus,
variant: variantProp, // deprecated. use `size` instead
visibleTokenCount,
Expand Down Expand Up @@ -259,17 +255,13 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
disabled={disabled}
hasLeadingVisual={Boolean(LeadingVisual || showLeadingLoadingIndicator)}
hasTrailingVisual={Boolean(TrailingVisual || showTrailingLoadingIndicator)}
width={widthProp}
minWidth={minWidthProp}
maxWidth={maxWidthProp}
size={inputSizeMap[size]}
validationStatus={validationStatus}
variant={variantProp} // deprecated. use `size` prop instead
onClick={focusInput}
data-token-wrapping={Boolean(preventTokenWrapping || maxHeight) || undefined}
className={clsx(className, styles.TextInputWrapper)}
style={maxHeight ? {maxHeight, ...style} : style}
sx={sxProp}
>
{IconComponent && !LeadingVisual && <IconComponent className="TextInput-icon" />}
<TextInputInnerVisualSlot
Expand Down
6 changes: 1 addition & 5 deletions packages/react/src/Textarea/Textarea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type {TextareaHTMLAttributes, ReactElement} from 'react'
import React from 'react'
import {TextInputBaseWrapper} from '../internal/components/TextInputWrapper'
import type {FormValidationStatus} from '../utils/types/FormValidationStatus'
import type {SxProp} from '../sx'
import classes from './TextArea.module.css'

export const DEFAULT_TEXTAREA_ROWS = 7
Expand Down Expand Up @@ -46,8 +45,7 @@ export type TextareaProps = {
* CSS styles to apply to the Textarea
*/
style?: React.CSSProperties
} & TextareaHTMLAttributes<HTMLTextAreaElement> &
SxProp
} & TextareaHTMLAttributes<HTMLTextAreaElement>

/**
* An accessible, native textarea component that supports validation states.
Expand All @@ -58,7 +56,6 @@ const Textarea = React.forwardRef<HTMLTextAreaElement, TextareaProps>(
{
value,
disabled,
sx: sxProp,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see usage of this in dotcom, are we planning to migrate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we would, but we're also planning to get rid of sx props in this upcoming release. Is there a plan for how to handle this? I know we have react-styled, but I don't really know what it is or how to use it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes I think react-styled is the way to go here. @joshblack would it be part of https://github.com/github/primer/issues/5422 as well to go in dotcom and replace the textarea + sx usage with react-styled instead or what's the pipeline here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, yeah. I would make sure in github-ui to update usage there first (the migration in this case is just find the places that use styled-system for these components and update the imports to pull the component from @primer/styled-react) and then merge here so that we don't have to deal with any challenges in the release.

Hope that makes sense, let me know if it'd be helpful to walk through this or do a loom or something since this is a new process 👀

required,
validationStatus,
rows = DEFAULT_TEXTAREA_ROWS,
Expand All @@ -76,7 +73,6 @@ const Textarea = React.forwardRef<HTMLTextAreaElement, TextareaProps>(
): ReactElement => {
return (
<TextInputBaseWrapper
sx={sxProp}
validationStatus={validationStatus}
disabled={disabled}
block={block}
Expand Down
Loading
Loading