Skip to content

Commit af09932

Browse files
AlessioGrAlessioGr
authored
fix(ui): use portals for popup to prevent clipping, improve keyboard navigation (#14910)
Previously, the Popup contents were displayed relative to the Popup trigger button. This caused the Popup contents to be hidden if the parent element has `overflow: hidden`, which was the case for doc_controls on smaller screen sizes (this PR adds an e2e test that previously failed). This PR refactors the Popup component to use `createPortal`, rendering the popup content directly to `document.body` to avoid clipping issues. ## Before https://github.com/user-attachments/assets/c1cc341a-2295-45b6-89d0-7e89866e5bfd ## After https://github.com/user-attachments/assets/2eb0d8b5-105e-468b-bd66-8ef8261a6306 While refactoring, I also improved the keyboard navigation for popups: | Feature | Before | After | |---------|--------|-------| | Tab cycling | Exited popup after last button | Cycles and goes back to first button | | Escape key | Did nothing | Closes popup and returns focus to trigger | | Arrow keys | Scrolled the page | Navigates between buttons (↑/↓) | | Button focus styling | Bad | Good | | `<a>` elements | Skipped during navigation | Included in keyboard navigation | ## Keyboard navigation before https://github.com/user-attachments/assets/9d212c90-7759-42cf-ad37-d7ee7dc64c5d ## Keyboard navigation after https://github.com/user-attachments/assets/dbc7d647-b58b-4e5b-928c-a6f3fdab6348
1 parent c20f6d4 commit af09932

28 files changed

Lines changed: 541 additions & 495 deletions

File tree

‎packages/ui/src/elements/ListControls/index.tsx‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import React, { Fragment, useEffect, useRef, useState } from 'react'
77

88
import type { ListControlsProps } from './types.js'
99

10-
import { Popup } from '../../elements/Popup/index.js'
10+
import { Popup, PopupList } from '../../elements/Popup/index.js'
1111
import { useUseTitleField } from '../../hooks/useUseAsTitle.js'
1212
import { ChevronIcon } from '../../icons/Chevron/index.js'
1313
import { Dots } from '../../icons/Dots/index.js'
@@ -210,9 +210,11 @@ export const ListControls: React.FC<ListControlsProps> = (props) => {
210210
size="small"
211211
verticalAlign="bottom"
212212
>
213-
{listMenuItems.map((item, i) => (
214-
<Fragment key={`list-menu-item-${i}`}>{item}</Fragment>
215-
))}
213+
<PopupList.ButtonGroup>
214+
{listMenuItems.map((item, i) => (
215+
<Fragment key={`list-menu-item-${i}`}>{item}</Fragment>
216+
))}
217+
</PopupList.ButtonGroup>
216218
</Popup>
217219
),
218220
].filter(Boolean)}

‎packages/ui/src/elements/Popup/PopupButtonList/index.scss‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
&:hover,
4848
&:focus-visible,
4949
&:focus-within {
50+
outline: none;
5051
background-color: var(--popup-button-highlight);
5152
}
5253
}

‎packages/ui/src/elements/Popup/PopupTrigger/index.tsx‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export type PopupTriggerProps = {
1212
className?: string
1313
disabled?: boolean
1414
noBackground?: boolean
15-
setActive: (active: boolean) => void
15+
setActive: (active: boolean, viaKeyboard?: boolean) => void
1616
size?: 'large' | 'medium' | 'small' | 'xsmall'
1717
}
1818

@@ -30,9 +30,16 @@ export const PopupTrigger: React.FC<PopupTriggerProps> = (props) => {
3030
.filter(Boolean)
3131
.join(' ')
3232

33-
const handleClick = React.useCallback(() => {
34-
setActive(!active)
35-
}, [active, setActive])
33+
const handleClick = () => {
34+
setActive(!active, false)
35+
}
36+
37+
const handleKeyDown = (e: React.KeyboardEvent) => {
38+
if (e.key === 'Enter' || e.key === ' ') {
39+
e.preventDefault()
40+
setActive(!active, true)
41+
}
42+
}
3643

3744
if (buttonType === 'none') {
3845
return null
@@ -43,11 +50,7 @@ export const PopupTrigger: React.FC<PopupTriggerProps> = (props) => {
4350
<div
4451
className={classes}
4552
onClick={handleClick}
46-
onKeyDown={(e) => {
47-
if (e.key === 'Enter') {
48-
handleClick()
49-
}
50-
}}
53+
onKeyDown={handleKeyDown}
5154
role="button"
5255
tabIndex={0}
5356
>
@@ -61,11 +64,7 @@ export const PopupTrigger: React.FC<PopupTriggerProps> = (props) => {
6164
className={classes}
6265
disabled={disabled}
6366
onClick={handleClick}
64-
onKeyDown={(e) => {
65-
if (e.key === 'Enter') {
66-
handleClick()
67-
}
68-
}}
67+
onKeyDown={handleKeyDown}
6968
tabIndex={0}
7069
type="button"
7170
>

‎packages/ui/src/elements/Popup/index.scss‎

Lines changed: 49 additions & 231 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,6 @@
22

33
@layer payload-default {
44
.popup {
5-
--popup-button-highlight: var(--theme-elevation-200);
6-
--popup-bg: var(--theme-input-bg);
7-
--popup-text: var(--theme-text);
8-
--popup-caret-size: 10px;
9-
--popup-x-padding: calc(var(--base) * 0.33);
10-
--popup-padding: calc(var(--base) * 0.5);
11-
--button-size-offset: -8px;
125
position: relative;
136

147
&__trigger-wrap {
@@ -18,248 +11,73 @@
1811
cursor: pointer;
1912
}
2013

21-
&__content {
22-
position: absolute;
23-
background: var(--popup-bg);
24-
opacity: 0;
25-
visibility: hidden;
26-
pointer-events: none;
27-
z-index: var(--z-popup);
28-
max-width: calc(100vw - #{$baseline});
29-
color: var(--popup-text);
30-
border-radius: 4px;
31-
padding-left: var(--popup-padding);
32-
padding-right: var(--popup-padding);
33-
min-width: var(--popup-width, auto);
34-
}
35-
36-
&__hide-scrollbar {
37-
overflow: hidden;
38-
}
39-
40-
&__scroll-container {
41-
overflow-y: auto;
42-
white-space: nowrap;
43-
width: calc(100% + var(--scrollbar-width));
44-
padding-top: var(--popup-padding);
45-
padding-bottom: var(--popup-padding);
46-
max-height: calc(var(--base) * 10);
47-
overflow-y: auto;
48-
}
49-
50-
&__scroll-content {
51-
width: calc(100% - var(--scrollbar-width));
52-
}
53-
54-
&--show-scrollbar {
55-
.popup__scroll-container,
56-
.popup__scroll-content {
57-
width: 100%;
58-
}
59-
}
60-
61-
&:focus,
62-
&:active {
63-
outline: none;
64-
}
65-
66-
////////////////////////////////
67-
// SIZE
68-
////////////////////////////////
69-
70-
&--size-xsmall {
71-
--popup-width: 80px;
72-
.popup__content {
73-
@include shadow-sm;
74-
}
75-
}
76-
77-
&--size-small {
78-
--popup-width: 100px;
79-
.popup__content {
80-
@include shadow-m;
81-
}
14+
&__on-hover-watch {
15+
display: contents;
8216
}
17+
}
8318

84-
&--size-medium {
85-
--popup-width: 150px;
86-
.popup__content {
87-
@include shadow-lg;
88-
}
89-
}
19+
.popup__hidden-content {
20+
display: none;
21+
}
9022

91-
&--size-large {
92-
--popup-width: 200px;
93-
.popup__content {
94-
@include shadow-lg;
95-
}
96-
}
23+
.popup__content {
24+
--popup-caret-size: 8px;
25+
--popup-button-highlight: var(--theme-elevation-150);
9726

98-
////////////////////////////////
99-
/// BUTTON SIZE
100-
////////////////////////////////
27+
position: absolute;
28+
z-index: var(--z-popup);
29+
background: var(--theme-input-bg);
30+
color: var(--theme-text);
31+
border-radius: 4px;
32+
padding: calc(var(--base) * 0.5);
33+
min-width: 150px;
34+
max-width: calc(100vw - var(--base));
35+
@include shadow-lg;
10136

102-
&--button-size-xsmall {
103-
--button-size-offset: -12px;
37+
&.popup--size-xsmall {
38+
min-width: 80px;
10439
}
105-
106-
&--button-size-small {
107-
--button-size-offset: -8px;
40+
&.popup--size-small {
41+
min-width: 100px;
10842
}
109-
110-
&--button-size-medium {
111-
--button-size-offset: -4px;
43+
&.popup--size-large {
44+
min-width: 200px;
11245
}
113-
114-
&--button-size-large {
115-
--button-size-offset: 0px;
46+
&.popup--size-fit-content {
47+
min-width: fit-content;
11648
}
49+
}
11750

118-
////////////////////////////////
119-
// HORIZONTAL ALIGNMENT
120-
////////////////////////////////
121-
[dir='rtl'] &--h-align-left {
122-
.popup__caret {
123-
right: var(--popup-padding);
124-
left: unset;
125-
}
126-
}
127-
&--h-align-left {
128-
.popup__caret {
129-
left: var(--popup-padding);
130-
}
131-
}
132-
&--h-align-center {
133-
.popup__content {
134-
left: 50%;
135-
transform: translateX(-50%);
136-
}
137-
138-
.popup__caret {
139-
left: 50%;
140-
transform: translateX(-50%);
141-
}
142-
}
143-
144-
[dir='rtl'] &--h-align-right {
145-
.popup__content {
146-
right: unset;
147-
left: 0;
148-
}
149-
150-
.popup__caret {
151-
right: unset;
152-
left: var(--popup-padding);
153-
}
154-
}
155-
156-
&--h-align-right {
157-
.popup__content {
158-
right: var(--button-size-offset);
159-
}
160-
161-
.popup__caret {
162-
right: var(--popup-padding);
163-
}
164-
}
165-
166-
////////////////////////////////
167-
// VERTICAL ALIGNMENT
168-
////////////////////////////////
169-
170-
&__caret {
171-
position: absolute;
172-
border: var(--popup-caret-size) solid transparent;
173-
}
174-
175-
&--v-align-top {
176-
.popup__content {
177-
@include shadow-lg;
178-
bottom: calc(100% + var(--popup-caret-size));
179-
}
180-
181-
.popup__caret {
182-
top: calc(100% - 1px);
183-
border-top-color: var(--popup-bg);
184-
}
185-
}
51+
.popup__scroll-container {
52+
overflow-y: auto;
53+
max-height: calc(var(--base) * 10);
18654

187-
&--v-align-bottom {
188-
.popup__content {
189-
@include shadow-lg-top;
190-
top: calc(100% + var(--popup-caret-size));
191-
}
55+
&:not(.popup__scroll-container--show-scrollbar) {
56+
scrollbar-width: none; // Firefox
57+
-ms-overflow-style: none; // IE/Edge
19258

193-
.popup__caret {
194-
bottom: calc(100% - 1px);
195-
border-bottom-color: var(--popup-bg);
59+
&::-webkit-scrollbar {
60+
display: none; // Chrome/Safari/Opera
19661
}
19762
}
63+
}
19864

199-
////////////////////////////////
200-
// ACTIVE
201-
////////////////////////////////
65+
.popup__caret {
66+
position: absolute;
67+
width: 0;
68+
height: 0;
69+
border: var(--popup-caret-size) solid transparent;
70+
left: var(--caret-left, 16px);
71+
transform: translateX(-50%);
20272

203-
&--active {
204-
.popup__content {
205-
opacity: 1;
206-
visibility: visible;
207-
pointer-events: all;
208-
}
73+
.popup--v-bottom & {
74+
top: calc(var(--popup-caret-size) * -2);
75+
border-bottom-color: var(--theme-input-bg);
20976
}
21077

211-
@include mid-break {
212-
--popup-padding: calc(var(--base) * 0.25);
213-
214-
&--h-align-center {
215-
.popup__content {
216-
left: 50%;
217-
transform: translateX(-0%);
218-
}
219-
220-
.popup__caret {
221-
left: 50%;
222-
transform: translateX(-0%);
223-
}
224-
}
225-
226-
&--h-align-right {
227-
.popup__content {
228-
right: 0;
229-
}
230-
231-
.popup__caret {
232-
right: var(--popup-padding);
233-
}
234-
}
235-
236-
&--force-h-align-left {
237-
.popup__content {
238-
left: 0;
239-
right: unset;
240-
transform: unset;
241-
}
242-
243-
.popup__caret {
244-
left: var(--popup-padding);
245-
right: unset;
246-
transform: unset;
247-
}
248-
}
249-
250-
&--force-h-align-right {
251-
.popup__content {
252-
right: 0;
253-
left: unset;
254-
transform: unset;
255-
}
256-
257-
.popup__caret {
258-
right: var(--popup-padding);
259-
left: unset;
260-
transform: unset;
261-
}
262-
}
78+
.popup--v-top & {
79+
bottom: calc(var(--popup-caret-size) * -2);
80+
border-top-color: var(--theme-input-bg);
26381
}
26482
}
26583
}

0 commit comments

Comments
 (0)