Skip to content

Commit 303ccef

Browse files
committed
feat(ui-buttons,ui-text-input): add condensed sizes to IconButton and simplify TextInput afterElement
Add condensedSmall and condensedMedium size options to IconButton, matching Button's existing condensed size support. These are intended for use inside compact layouts like TextInput. Remove temporary workarounds from TextInput that were needed to make the old IconButton work inside the afterElement slot: - Remove adjustAfterElementHeight DOM hack - Remove sizeVariants spread on afterElement container - Remove afterElementHasWidth state machinery - Simplify afterElement to a basic flex container with padding Update documentation examples in both IconButton and TextInput READMEs with a password visibility toggle use case and current icon names.
1 parent 83f9117 commit 303ccef

6 files changed

Lines changed: 77 additions & 144 deletions

File tree

packages/ui-buttons/src/IconButton/v2/README.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,52 @@ type: example
1212
<IconButton screenReaderLabel="Add User"><PlusInstUIIcon /></IconButton>
1313
```
1414

15+
### Sizes
16+
17+
```js
18+
---
19+
type: example
20+
---
21+
<View display="block">
22+
<IconButton size="small" screenReaderLabel="Add" margin="small"><PlusInstUIIcon /></IconButton>
23+
<IconButton size="medium" screenReaderLabel="Add" margin="small"><PlusInstUIIcon /></IconButton>
24+
<IconButton size="large" screenReaderLabel="Add" margin="small"><PlusInstUIIcon /></IconButton>
25+
</View>
26+
```
27+
28+
There are also two condensed size variants for compact layouts: `condensedSmall` and `condensedMedium`. These are designed to be used inside other components, such as a [TextInput](TextInput).
29+
30+
```js
31+
---
32+
type: example
33+
---
34+
const PasswordInput = () => {
35+
const [showPassword, setShowPassword] = useState(false)
36+
37+
return (
38+
<View as="div" maxWidth="20rem">
39+
<TextInput
40+
renderLabel="Password"
41+
type={showPassword ? 'text' : 'password'}
42+
renderAfterInput={
43+
<IconButton
44+
size="condensedMedium"
45+
withBackground={false}
46+
withBorder={false}
47+
screenReaderLabel={showPassword ? 'Hide password' : 'Show password'}
48+
onClick={() => setShowPassword(!showPassword)}
49+
>
50+
{showPassword ? <EyeOffInstUIIcon /> : <EyeInstUIIcon />}
51+
</IconButton>
52+
}
53+
/>
54+
</View>
55+
)
56+
}
57+
58+
render(<PasswordInput />)
59+
```
60+
1561
### Accessibility
1662

1763
Because the IconButton visually only renders an icon, a description is necessary for assistive technologies. The `screenReaderLabel` prop is required for this purpose, and should consist of a complete description of the action.

packages/ui-buttons/src/IconButton/v2/props.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type IconButtonOwnProps = {
5959
/**
6060
* The size of the `IconButton`
6161
*/
62-
size?: 'small' | 'medium' | 'large'
62+
size?: 'small' | 'medium' | 'large' | 'condensedSmall' | 'condensedMedium'
6363

6464
/**
6565
* Provides a reference to the `IconButton`'s underlying html element.

packages/ui-text-input/src/TextInput/v2/README.md

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ type: example
164164
render(<ExtraContentExample />)
165165
```
166166

167-
Another common usecase is to add an `IconButton` at the end of a TextInput, e.g. for revealing the content of a password field. In these cases, please use the `withBorder={false}` and `withBackground={false}` props for the IconButton.
167+
Another common usecase is to add an [IconButton](IconButton) at the end of a TextInput, e.g. for revealing the content of a password field. Use the `condensedMedium` size together with `withBorder={false}` and `withBackground={false}` on the IconButton.
168168

169169
```js
170170
---
@@ -174,18 +174,25 @@ const InputsWithButtonsExample = () => {
174174
const [passwordValue, setPasswordValue] = useState('')
175175
const [showPassword, setShowPassword] = useState(false)
176176
return (
177-
<TextInput
178-
renderLabel="Password"
179-
type={showPassword ? 'text' : 'password'}
180-
placeholder="Find something..."
181-
value={passwordValue}
182-
onChange={(e, newValue) => setPasswordValue(newValue)}
183-
renderAfterInput={
184-
<IconButton withBorder={false} withBackground={false} onClick={() => setShowPassword(prevState => !prevState)} screenReaderLabel={showPassword ? 'Hide password' : 'Show password'}>
185-
{showPassword ? <IconOffLine/> : <IconEyeLine/>}
186-
</IconButton>
187-
}
188-
/>
177+
<View as="div" maxWidth="20rem">
178+
<TextInput
179+
renderLabel="Password"
180+
type={showPassword ? 'text' : 'password'}
181+
value={passwordValue}
182+
onChange={(e, newValue) => setPasswordValue(newValue)}
183+
renderAfterInput={
184+
<IconButton
185+
size="condensedMedium"
186+
withBorder={false}
187+
withBackground={false}
188+
screenReaderLabel={showPassword ? 'Hide password' : 'Show password'}
189+
onClick={() => setShowPassword((prev) => !prev)}
190+
>
191+
{showPassword ? <EyeOffInstUIIcon /> : <EyeInstUIIcon />}
192+
</IconButton>
193+
}
194+
/>
195+
</View>
189196
)
190197
}
191198
render(<InputsWithButtonsExample />)
@@ -228,7 +235,7 @@ type: example
228235
<View as="div" maxWidth="250px">
229236
<TextInput
230237
renderLabel="I will not wrap"
231-
renderBeforeInput={() => (<IconSearchLine inline={false} />)}
238+
renderBeforeInput={<SearchInstUIIcon />}
232239
renderAfterInput={<Avatar name="Paula Panda" src={avatarSquare} size="x-small" />}
233240
shouldNotWrap
234241
/>

packages/ui-text-input/src/TextInput/v2/index.tsx

Lines changed: 5 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,12 @@ import {
3131
withDeterministicId,
3232
safeCloneElement
3333
} from '@instructure/ui-react-utils'
34-
import {
35-
isActiveElement,
36-
addEventListener,
37-
getCSSStyleDeclaration
38-
} from '@instructure/ui-dom-utils'
34+
import { isActiveElement, addEventListener } from '@instructure/ui-dom-utils'
3935
import { FormField } from '@instructure/ui-form-field/latest'
4036
import { withStyle } from '@instructure/emotion'
4137

4238
import generateStyle from './styles'
43-
import type {
44-
TextInputProps,
45-
TextInputState,
46-
TextInputStyleProps
47-
} from './props'
39+
import type { TextInputProps, TextInputStyleProps } from './props'
4840
import { allowedProps } from './props'
4941
import type { Renderable } from '@instructure/shared-types'
5042

@@ -56,7 +48,7 @@ tags: form, field, input
5648
**/
5749
@withDeterministicId()
5850
@withStyle(generateStyle)
59-
class TextInput extends Component<TextInputProps, TextInputState> {
51+
class TextInput extends Component<TextInputProps> {
6052
static readonly componentId = 'TextInput'
6153

6254
static allowedProps = allowedProps
@@ -75,15 +67,13 @@ class TextInput extends Component<TextInputProps, TextInputState> {
7567

7668
constructor(props: TextInputProps) {
7769
super(props)
78-
this.state = { afterElementHasWidth: undefined }
7970
this._defaultId = props.deterministicId!()
8071
this._messagesId = props.deterministicId!('TextInput-messages')
8172
}
8273

8374
ref: Element | null = null
8475

8576
private _input: HTMLInputElement | null = null
86-
private _afterElement: HTMLSpanElement | null = null
8777

8878
private readonly _defaultId: string
8979
private readonly _messagesId: string
@@ -107,11 +97,7 @@ class TextInput extends Component<TextInputProps, TextInputState> {
10797
'focus',
10898
this.handleFocus
10999
)
110-
this.setState({
111-
afterElementHasWidth: this.getElementHasWidth(this._afterElement)
112-
})
113100
}
114-
this.adjustAfterElementHeight()
115101
this.props.makeStyles?.(this.makeStyleProps())
116102
}
117103

@@ -121,12 +107,7 @@ class TextInput extends Component<TextInputProps, TextInputState> {
121107
}
122108
}
123109

124-
componentDidUpdate(prevProps: TextInputProps) {
125-
if (prevProps.renderAfterInput !== this.props.renderAfterInput) {
126-
this.setState({
127-
afterElementHasWidth: this.getElementHasWidth(this._afterElement)
128-
})
129-
}
110+
componentDidUpdate() {
130111
this.props.makeStyles?.(this.makeStyleProps())
131112
}
132113

@@ -152,37 +133,7 @@ class TextInput extends Component<TextInputProps, TextInputState> {
152133
return rendered
153134
}
154135

155-
adjustAfterElementHeight() {
156-
const afterElementChild = this._afterElement
157-
?.firstElementChild as HTMLElement | null
158-
159-
// Check if the child is a button, then get the button's first child (the content span)
160-
const buttonContentSpan =
161-
afterElementChild?.tagName === 'BUTTON'
162-
? (afterElementChild.firstElementChild as HTMLElement | null)
163-
: null
164-
165-
// This is a necessary workaround for DateInput2 because it uses a Popover, which has a nested Button as an afterElement
166-
// Check if the child is a Popover's inner span containing a button, then get the button's first child (the content span)
167-
const popoverContentSpan =
168-
afterElementChild?.tagName === 'SPAN' &&
169-
afterElementChild.firstElementChild?.tagName === 'BUTTON'
170-
? (afterElementChild.firstElementChild
171-
.firstElementChild as HTMLElement | null)
172-
: null
173-
174-
const targetSpan = buttonContentSpan ?? popoverContentSpan
175-
176-
if (targetSpan) {
177-
// Set the height to 36px (the height of a medium TextInput) to avoid layout shift when the afterElement content changes
178-
// this temporary workaround is necessary because otherwise the layout breaks, later on IconButton's default height will be adjusted to the TextInput size
179-
// so this workaround will not be needed anymore
180-
targetSpan.style.height = '36px'
181-
}
182-
}
183-
184136
makeStyleProps = (): TextInputStyleProps => {
185-
const { afterElementHasWidth } = this.state
186137
const beforeElement = this.props.renderBeforeInput
187138
? callRenderProp(this.props.renderBeforeInput)
188139
: null
@@ -193,7 +144,6 @@ class TextInput extends Component<TextInputProps, TextInputState> {
193144
interaction: this.interaction,
194145
invalid: this.invalid,
195146
success: success,
196-
afterElementHasWidth: afterElementHasWidth,
197147
beforeElementExists: !!beforeElement
198148
}
199149
}
@@ -303,34 +253,6 @@ class TextInput extends Component<TextInputProps, TextInputState> {
303253
)
304254
}
305255

306-
getElementHasWidth(element: Element | null) {
307-
if (!element) {
308-
return undefined
309-
}
310-
311-
const computedStyle = getCSSStyleDeclaration(element)
312-
if (!computedStyle) {
313-
return undefined
314-
}
315-
316-
const { width, paddingInlineStart, paddingInlineEnd } = computedStyle
317-
318-
if (width === 'auto' || width === '') {
319-
// This is a workaround for an edge-case, when the TextInput's parent
320-
// is hidden on load, so the element is not visible either.
321-
// In this case the computed width is going to be either 'auto' or '',
322-
// so we assume it has width so that the padding won't be removed.
323-
return true
324-
}
325-
326-
const elementWidth =
327-
parseFloat(width) -
328-
parseFloat(paddingInlineStart) -
329-
parseFloat(paddingInlineEnd)
330-
331-
return elementWidth > 0
332-
}
333-
334256
render() {
335257
const {
336258
width,
@@ -381,14 +303,7 @@ class TextInput extends Component<TextInputProps, TextInputState> {
381303
<span css={styles?.inputLayout}>
382304
{this.renderInput()}
383305
{afterElement && (
384-
<span
385-
css={styles?.afterElement}
386-
ref={(e) => {
387-
this._afterElement = e
388-
}}
389-
>
390-
{afterElement}
391-
</span>
306+
<span css={styles?.afterElement}>{afterElement}</span>
392307
)}
393308
</span>
394309
</span>

packages/ui-text-input/src/TextInput/v2/props.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -223,22 +223,12 @@ const allowedProps: AllowedPropKeys = [
223223
'margin'
224224
]
225225

226-
type TextInputState = {
227-
afterElementHasWidth?: boolean
228-
}
229-
230226
type TextInputStyleProps = {
231227
interaction: InteractionType
232228
success: boolean
233229
invalid: boolean
234-
afterElementHasWidth: TextInputState['afterElementHasWidth']
235230
beforeElementExists: boolean
236231
}
237232

238-
export type {
239-
TextInputProps,
240-
TextInputState,
241-
TextInputStyleProps,
242-
TextInputStyle
243-
}
233+
export type { TextInputProps, TextInputStyleProps, TextInputStyle }
244234
export { allowedProps }

packages/ui-text-input/src/TextInput/v2/styles.ts

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,7 @@ const generateStyle = (
4444
state: TextInputStyleProps
4545
): TextInputStyle => {
4646
const { size, textAlign, shouldNotWrap } = props
47-
const {
48-
interaction,
49-
success,
50-
invalid,
51-
afterElementHasWidth,
52-
beforeElementExists
53-
} = state
47+
const { interaction, success, invalid, beforeElementExists } = state
5448

5549
const sizeVariants = {
5650
small: {
@@ -212,32 +206,13 @@ const generateStyle = (
212206
flexDirection: 'row'
213207
},
214208
afterElement: {
215-
// the next couple lines (until the `label`) is needed so the IconButton looks OK inside the TextInput
216-
// explanation: if the content inside is not a button or a popover (which could contain a button) it should have some padding on the right
217-
// lineHeight is only needed if it is not popover or button
218-
'& > :not(button):not([data-position^="Popover"])': {
219-
marginRight: paddingHorizontalVariants[size!]
220-
// TODO check if it looks OK with the new buttons. With this it does not look OK with new icons
221-
//...(sizeVariants[size!] && {
222-
// lineHeight: sizeVariants[size!]?.lineHeight
223-
//})
224-
},
225209
display: 'flex',
226210
alignItems: 'center',
227-
// Spread all sizeVariants except lineHeight (handled above)
228-
...(sizeVariants[size!]
229-
? // eslint-disable-next-line @typescript-eslint/no-unused-vars
230-
(({ lineHeight, ...rest }) => rest)(sizeVariants[size!])
231-
: {}),
211+
paddingInlineEnd: paddingHorizontalVariants[size!],
232212
label: 'textInput__afterElement',
233213
...viewBase,
234214
borderRadius: componentTheme.borderRadius,
235-
flexShrink: 0,
236-
// we only override the padding once the width is calculated,
237-
// it needs the padding on render
238-
...(afterElementHasWidth === false && {
239-
paddingInlineEnd: 0
240-
})
215+
flexShrink: 0
241216
}
242217
}
243218
}

0 commit comments

Comments
 (0)