Skip to content

Commit b1bc451

Browse files
committed
refactor(ui-view): refactor View focus ring code
it uses now the outline CSS prop instead of a :before pseudo class. This has several advantages: - View can now display focus rings even if the position is not absolute - focus rings follow border settings if they are not the same for all corners page CSS is simpler Also add a new prop that will be useful when we use this code for all components that need to display a focus ring
1 parent 32bb431 commit b1bc451

File tree

5 files changed

+187
-231
lines changed

5 files changed

+187
-231
lines changed

packages/ui-flex/src/Flex/README.md

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -279,25 +279,28 @@ type: example
279279
### Handling overflow
280280

281281
When `direction` is set to `column`, Flex.Items' `overflowY` property is automagically set
282-
to `auto` to account for content overflow with a vertical scrollbar.
282+
to `auto` to account for content overflow with a vertical scrollbar. Add padding, so focus rings are not cut off.
283283

284284
> To override this default, simply set `overflowY` on the Flex.Item to either `visible` or `hidden`.
285285
286286
```js
287287
---
288288
type: example
289289
---
290-
<Flex
291-
withVisualDebug
292-
direction="column"
293-
>
294-
<Flex.Item padding="small">
295-
<Heading>Pandas are cute, right?</Heading>
296-
</Flex.Item>
297-
<Flex.Item size="150px" padding="small">
298-
<Img src={avatarSquare} />
299-
</Flex.Item>
300-
</Flex>
290+
<Flex
291+
withVisualDebug
292+
direction="column"
293+
>
294+
<Flex.Item padding="small">
295+
<Heading>Pandas are cute, right?</Heading>
296+
</Flex.Item>
297+
<Flex.Item>
298+
<TextInput name="name" renderLabel="If you dont add padding, the focus ring will be cut off!" />
299+
</Flex.Item>
300+
<Flex.Item size="150px" padding="small">
301+
<Img src={avatarSquare} />
302+
</Flex.Item>
303+
</Flex>
301304
```
302305
303306
### A few common layouts

packages/ui-view/src/View/README.md

Lines changed: 87 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -398,9 +398,6 @@ type: example
398398
`position` sets the CSS position rule for the component: `static`, `absolute`, `relative`,
399399
`sticky`, or `fixed`.
400400

401-
> Note that `position="sticky"` is currently [not as fully supported](https://caniuse.com/#feat=css-sticky)
402-
> as the other values.
403-
404401
```js
405402
---
406403
type: example
@@ -428,25 +425,19 @@ type: example
428425

429426
By default, if a `View` is rendered as a focusable element, a focus outline will display when it is focused for accessibility.
430427

431-
> Note that `position` must be set to `relative` for the focus ring to display.
432-
> (This restriction exists because the focus ring requires styling a pseudo element
433-
> that has absolute positioning.)
434-
435428
```javascript
436429
---
437430
type: example
438431
---
439-
<View
440-
position="relative"
441-
tabIndex="0"
442-
role="button"
443-
cursor="pointer"
444-
display="block"
445-
margin="large"
446-
padding="small"
447-
>
448-
Tab here to see the focus outline
449-
</View>
432+
<Flex gap="medium" direction="column">
433+
<View tabIndex="0" role="button" cursor="pointer">
434+
Tab here to see the focus outline
435+
</View>
436+
<View focusWithin>
437+
if the <code>focusWithin</code> prop is <code>true</code>, the View will display the focus ring if any of its descendants receives focus
438+
<div tabindex="0" role="button" style={{outline: 'none'}}>Tab here to see the focus outline</div>
439+
</View>
440+
</Flex>
450441
```
451442

452443
In some situations, you may want to manually control when the focus outline is displayed instead of leaving it up to the browser.
@@ -455,8 +446,7 @@ Be careful when overriding the display of the focus outline as it is essential f
455446

456447
The focus outline adjusts to account for the shape of the View. For example, the following values can be set for `borderRadius`:
457448
`circle`, `pill`, `small`, `medium`, and `large`. In each case, the border radius of the focus outline will automatically adjust
458-
to match the border radius of the corresponding View. For Views with irregular border radius (e.g., `borderRadius="small large none medium"`), the focus outline will appear with square edges. The color of the focus outline can be
459-
changed for different contexts via the `focusColor` property.
449+
to match the border radius of the corresponding View. The color of the focus outline can be changed for different contexts via the `focusColor` property.
460450

461451
- ```javascript
462452
class FocusedExample extends React.Component {
@@ -465,7 +455,8 @@ changed for different contexts via the `focusColor` property.
465455

466456
this.state = {
467457
isFocused: true,
468-
inset: false
458+
inset: false,
459+
focusColor: undefined
469460
}
470461
}
471462

@@ -477,6 +468,10 @@ changed for different contexts via the `focusColor` property.
477468
this.setState({ inset: event.target.checked })
478469
}
479470

471+
updateFocusRingColor = (event, value) => {
472+
this.setState({ focusColor: value })
473+
}
474+
480475
render() {
481476
return (
482477
<View as="div">
@@ -495,16 +490,32 @@ changed for different contexts via the `focusColor` property.
495490
</ScreenReaderContent>
496491
}
497492
>
498-
<Checkbox
499-
label="withFocusOutline"
500-
checked={this.state.isFocused}
501-
onChange={this.updateFocused}
502-
/>
503-
<Checkbox
504-
label="focusPosition = inset"
505-
checked={this.state.inset}
506-
onChange={this.updateInset}
507-
/>
493+
<Flex gap="small" direction="row">
494+
<Flex gap="small" direction="column" width="15rem">
495+
<Checkbox
496+
label="withFocusOutline"
497+
checked={this.state.isFocused}
498+
onChange={this.updateFocused}
499+
/>
500+
<Checkbox
501+
label="focusPosition = inset"
502+
checked={this.state.inset}
503+
onChange={this.updateInset}
504+
/>
505+
</Flex>
506+
<RadioInputGroup
507+
onChange={this.updateFocusRingColor}
508+
name="focusColor"
509+
defaultValue="info"
510+
variant="toggle"
511+
description="Focus ring color"
512+
>
513+
<RadioInput label="info" value="info" />
514+
<RadioInput label="inverse" value="inverse" />
515+
<RadioInput label="success" value="success" />
516+
<RadioInput label="danger" value="danger" />
517+
</RadioInputGroup>
518+
</Flex>
508519
</FormFieldGroup>
509520
</View>
510521
<View as="div">
@@ -517,6 +528,7 @@ changed for different contexts via the `focusColor` property.
517528
borderRadius="small"
518529
borderWidth="small"
519530
position="relative"
531+
focusColor={this.state.focusColor}
520532
withFocusOutline={this.state.isFocused}
521533
focusPosition={this.state.inset ? 'inset' : 'offset'}
522534
>
@@ -530,6 +542,7 @@ changed for different contexts via the `focusColor` property.
530542
borderRadius="medium"
531543
borderWidth="small"
532544
position="relative"
545+
focusColor={this.state.focusColor}
533546
withFocusOutline={this.state.isFocused}
534547
focusPosition={this.state.inset ? 'inset' : 'offset'}
535548
>
@@ -543,6 +556,7 @@ changed for different contexts via the `focusColor` property.
543556
borderRadius="large"
544557
borderWidth="small"
545558
position="relative"
559+
focusColor={this.state.focusColor}
546560
withFocusOutline={this.state.isFocused}
547561
focusPosition={this.state.inset ? 'inset' : 'offset'}
548562
>
@@ -557,6 +571,7 @@ changed for different contexts via the `focusColor` property.
557571
borderRadius="circle"
558572
borderWidth="small"
559573
position="relative"
574+
focusColor={this.state.focusColor}
560575
withFocusOutline={this.state.isFocused}
561576
focusPosition={this.state.inset ? 'inset' : 'offset'}
562577
>
@@ -583,7 +598,7 @@ changed for different contexts via the `focusColor` property.
583598
borderWidth="small"
584599
position="relative"
585600
withFocusOutline={this.state.isFocused}
586-
focusColor="inverse"
601+
focusColor={this.state.focusColor}
587602
focusPosition={this.state.inset ? 'inset' : 'offset'}
588603
>
589604
medium
@@ -597,9 +612,9 @@ changed for different contexts via the `focusColor` property.
597612
borderRadius="pill"
598613
borderWidth="small"
599614
position="relative"
600-
focusColor="success"
601615
width="100px"
602616
textAlign="center"
617+
focusColor={this.state.focusColor}
603618
withFocusOutline={this.state.isFocused}
604619
focusPosition={this.state.inset ? 'inset' : 'offset'}
605620
>
@@ -612,8 +627,8 @@ changed for different contexts via the `focusColor` property.
612627
background="primary"
613628
borderWidth="small"
614629
borderRadius="none large"
615-
focusColor="danger"
616630
position="relative"
631+
focusColor={this.state.focusColor}
617632
withFocusOutline={this.state.isFocused}
618633
focusPosition={this.state.inset ? 'inset' : 'offset'}
619634
>
@@ -624,17 +639,18 @@ changed for different contexts via the `focusColor` property.
624639
)
625640
}
626641
}
627-
628642
render(<FocusedExample />)
629643
```
630644

631645
- ```javascript
632646
const FocusedExample = () => {
633647
const [isFocused, setIsFocused] = useState(true)
634648
const [inset, setInset] = useState(false)
649+
const [focusColor, setfocusColor] = useState(undefined)
635650

636651
const updateFocused = (event) => setIsFocused(event.target.checked)
637652
const updateInset = (event) => setInset(event.target.checked)
653+
const updateFocusRingColor = (event) => setfocusColor(event.target.value)
638654

639655
return (
640656
<View as="div">
@@ -653,16 +669,32 @@ changed for different contexts via the `focusColor` property.
653669
</ScreenReaderContent>
654670
}
655671
>
656-
<Checkbox
657-
label="withFocusOutline"
658-
checked={isFocused}
659-
onChange={updateFocused}
660-
/>
661-
<Checkbox
662-
label="focusPosition = inset"
663-
checked={inset}
664-
onChange={updateInset}
665-
/>
672+
<Flex gap="small" direction="row">
673+
<Flex gap="small" direction="column" width="15rem">
674+
<Checkbox
675+
label="withFocusOutline"
676+
checked={isFocused}
677+
onChange={updateFocused}
678+
/>
679+
<Checkbox
680+
label="focusPosition = inset"
681+
checked={inset}
682+
onChange={updateInset}
683+
/>
684+
</Flex>
685+
<RadioInputGroup
686+
onChange={updateFocusRingColor}
687+
name="focusColor_2"
688+
defaultValue="info"
689+
variant="toggle"
690+
description="Focus ring color"
691+
>
692+
<RadioInput label="info" value="info" />
693+
<RadioInput label="inverse" value="inverse" />
694+
<RadioInput label="success" value="success" />
695+
<RadioInput label="danger" value="danger" />
696+
</RadioInputGroup>
697+
</Flex>
666698
</FormFieldGroup>
667699
</View>
668700
<View as="div">
@@ -675,6 +707,7 @@ changed for different contexts via the `focusColor` property.
675707
borderRadius="small"
676708
borderWidth="small"
677709
position="relative"
710+
focusColor={focusColor}
678711
withFocusOutline={isFocused}
679712
focusPosition={inset ? 'inset' : 'offset'}
680713
>
@@ -689,6 +722,7 @@ changed for different contexts via the `focusColor` property.
689722
borderWidth="small"
690723
position="relative"
691724
withFocusOutline={isFocused}
725+
focusColor={focusColor}
692726
focusPosition={inset ? 'inset' : 'offset'}
693727
>
694728
medium
@@ -702,6 +736,7 @@ changed for different contexts via the `focusColor` property.
702736
borderWidth="small"
703737
position="relative"
704738
withFocusOutline={isFocused}
739+
focusColor={focusColor}
705740
focusPosition={inset ? 'inset' : 'offset'}
706741
>
707742
large
@@ -716,6 +751,7 @@ changed for different contexts via the `focusColor` property.
716751
borderWidth="small"
717752
position="relative"
718753
withFocusOutline={isFocused}
754+
focusColor={focusColor}
719755
focusPosition={inset ? 'inset' : 'offset'}
720756
>
721757
<Flex
@@ -741,6 +777,7 @@ changed for different contexts via the `focusColor` property.
741777
borderWidth="small"
742778
position="relative"
743779
withFocusOutline={isFocused}
780+
focusColor={focusColor}
744781
focusColor="inverse"
745782
focusPosition={inset ? 'inset' : 'offset'}
746783
>
@@ -758,6 +795,7 @@ changed for different contexts via the `focusColor` property.
758795
focusColor="success"
759796
width="100px"
760797
textAlign="center"
798+
focusColor={focusColor}
761799
withFocusOutline={isFocused}
762800
focusPosition={inset ? 'inset' : 'offset'}
763801
>
@@ -772,6 +810,7 @@ changed for different contexts via the `focusColor` property.
772810
borderRadius="none large"
773811
focusColor="danger"
774812
position="relative"
813+
focusColor={focusColor}
775814
withFocusOutline={isFocused}
776815
focusPosition={inset ? 'inset' : 'offset'}
777816
>
@@ -936,7 +975,9 @@ props.
936975

937976
### Debugging
938977

939-
Set the `withVisualDebug` prop to see the View's boundaries.
978+
Set the `withVisualDebug` prop to see the View's boundaries. Use this only for debugging.
979+
980+
> This effect uses a CSS box-shadow, so the `shadow` prop will be overridden
940981
941982
```js
942983
---

packages/ui-view/src/View/__new-tests__/View.test.tsx

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -234,39 +234,6 @@ describe('<View />', () => {
234234
expect(newStyles.maxWidth).toEqual('200px')
235235
})
236236

237-
describe('withFocusOutline', () => {
238-
it('should warn when withFocusOutline is true without position=relative', () => {
239-
render(
240-
<View withFocusOutline>
241-
<h1>View Content</h1>
242-
</View>
243-
)
244-
const expectedErrorMessage =
245-
'Warning: [View] the focus outline will only show if the `position` prop is `relative`.'
246-
247-
expect(consoleErrorMock).toHaveBeenCalledWith(
248-
expect.stringContaining(expectedErrorMessage),
249-
expect.any(String)
250-
)
251-
})
252-
253-
it('should warn when withFocusOutline is `true`, display is set to `inline`, and focusPosition is set to `offset`', () => {
254-
render(
255-
<View withFocusOutline display="inline" focusPosition="offset">
256-
<h1>View Content</h1>
257-
</View>
258-
)
259-
260-
const expectedErrorMessage =
261-
'Warning: [View] when display is set to `inline` the focus outline will only show if `focusPosition` is set to `inset`.'
262-
263-
expect(consoleErrorMock).toHaveBeenCalledWith(
264-
expect.stringContaining(expectedErrorMessage),
265-
expect.any(String)
266-
)
267-
})
268-
})
269-
270237
it('should meet a11y standards', async () => {
271238
const { container } = render(<View>View Content</View>)
272239

0 commit comments

Comments
 (0)