Skip to content

Commit 4614fe6

Browse files
Fix #1652 - Listbox popover does not move when page scrolls (#1747)
* Fix frozen listbox popover * Update z-indexes on a few items, aligned with suggested z-index protocol * Handle Listbox in SidebarModal * Add z-index for MswBanner * migrate to useContext; add tests * Move zIndex into Tailwind theme extension; refactor * make the e2e tests truly evil --------- Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
1 parent 14685b1 commit 4614fe6

File tree

14 files changed

+215
-71
lines changed

14 files changed

+215
-71
lines changed

app/components/MswBanner.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export function MswBanner() {
3535
return (
3636
<>
3737
{/* The [&+*]:pt-10 style is to ensure the page container isn't pushed out of screen as it uses 100vh for layout */}
38-
<label className="absolute flex h-10 w-full items-center justify-center text-sans-md text-info-secondary bg-info-secondary [&+*]:pt-10">
38+
<label className="absolute z-topBar flex h-10 w-full items-center justify-center text-sans-md text-info-secondary bg-info-secondary [&+*]:pt-10">
3939
<Info16Icon className="mr-2" /> This is a technical preview.
4040
<button
4141
className="ml-2 flex items-center gap-0.5 text-sans-md hover:text-info"

app/components/ToastStack.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export function ToastStack() {
2424
})
2525

2626
return (
27-
<div className="pointer-events-auto fixed bottom-4 left-4 z-50 flex flex-col items-end space-y-2">
27+
<div className="pointer-events-auto fixed bottom-4 left-4 z-toast flex flex-col items-end space-y-2">
2828
{transition((style, item) => (
2929
<animated.div
3030
style={{

app/components/TopBar.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export function TopBar({ children }: { children: React.ReactNode }) {
5050
</div>
5151
{/* Height is governed by PageContainer grid */}
5252
{/* shrink-0 is needed to prevent getting squished by body content */}
53-
<div className="border-b bg-default border-secondary">
53+
<div className="z-topBar border-b bg-default border-secondary">
5454
<div className="mx-3 flex h-[60px] shrink-0 items-center justify-between">
5555
<div className="flex items-center">{otherPickers}</div>
5656
<div>

app/components/form/fields/DateTimeRangePicker.spec.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
import { getLocalTimeZone, now as getNow } from '@internationalized/date'
99
import { fireEvent, render, screen } from '@testing-library/react'
10+
import ResizeObserverPolyfill from 'resize-observer-polyfill'
1011
import { beforeAll, describe, expect, it, vi } from 'vitest'
1112

1213
import { clickByRole } from 'app/test/unit'
@@ -34,6 +35,8 @@ function renderLastDay() {
3435
}
3536

3637
beforeAll(() => {
38+
global.ResizeObserver = ResizeObserverPolyfill
39+
3740
vi.useFakeTimers()
3841
vi.setSystemTime(now.toDate())
3942

app/test/e2e/utils.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,23 @@ export async function getDevUserPage(browser: Browser): Promise<Page> {
120120
])
121121
return await browserContext.newPage()
122122
}
123+
124+
/**
125+
* Assert that the item is visible and in the viewport but obscured by something
126+
* else, as indicated by it not being clickable. In order to avoid false
127+
* positives where something is not clickable due to it being not attached or
128+
* something, we assert visible and in viewport first.
129+
*/
130+
export async function expectObscured(locator: Locator) {
131+
// counterintuitively, expect visible does not mean actually visible, it just
132+
// means attached and not having display: none
133+
await expect(locator).toBeVisible()
134+
await expect(locator).toBeInViewport()
135+
136+
// Attempt click with `trial: true`, which means only the actionability checks
137+
// run but the click does not actually happen. Short timeout means this will
138+
// fail fast if not clickable.
139+
await expect(
140+
async () => await locator.click({ trial: true, timeout: 50 })
141+
).rejects.toThrow(/locator.click: Timeout 50ms exceeded/)
142+
}

app/test/e2e/z-index.e2e.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* This Source Code Form is subject to the terms of the Mozilla Public
3+
* License, v. 2.0. If a copy of the MPL was not distributed with this
4+
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
5+
*
6+
* Copyright Oxide Computer Company
7+
*/
8+
import { expect, test } from '@playwright/test'
9+
10+
import { expectObscured } from './utils'
11+
12+
test('Dropdown content can scroll off page and doesn’t hide TopBar', async ({ page }) => {
13+
// load the page
14+
await page.goto('/utilization')
15+
await expect(page.getByText('Capacity & Utilization')).toBeVisible()
16+
17+
const button = page.getByRole('button', { name: 'All projects' })
18+
19+
// click on the 'All projects' dropdown
20+
await button.click()
21+
const options = ['All projects', 'mock-project', 'other-project']
22+
for (const name of options) {
23+
const option = page.getByRole('option', { name })
24+
await expect(option).toBeVisible()
25+
await expect(option).toBeInViewport()
26+
}
27+
28+
// scroll the page down by 275px
29+
await page.mouse.wheel(0, 275)
30+
31+
// if we don't do this, the test doesn't wait long enough for the following
32+
// assertions to become true
33+
await expect(button).not.toBeInViewport()
34+
35+
// now the top the listbox option is obscured by the topbar
36+
await expectObscured(page.getByRole('option', { name: 'All projects' }))
37+
38+
// but we can still click the bottom one
39+
await page.getByRole('option', { name: 'other-project' }).click()
40+
await expect(page.getByRole('button', { name: 'other-project' })).toBeVisible()
41+
})
42+
43+
test('Dropdown content in SidebarModal shows on screen', async ({ page }) => {
44+
// go to an instance’s Network Interfaces page
45+
await page.goto('/projects/mock-project/instances/db1/network-interfaces')
46+
47+
// stop the instance
48+
await page.getByRole('button', { name: 'Instance actions' }).click()
49+
await page.getByRole('menuitem', { name: 'Stop' }).click()
50+
51+
// open the add network interface side modal
52+
await page.getByRole('button', { name: 'Add network interface' }).click()
53+
54+
// fill out the form
55+
await page.getByLabel('Name').fill('alt-nic')
56+
57+
// select the VPC and subnet via the dropdowns. The fact that the options are
58+
// clickable means they are not obscured due to having a too-low z-index
59+
await page.getByRole('button', { name: 'VPC' }).click()
60+
await page.getByRole('option', { name: 'mock-vpc' }).click()
61+
await page.getByRole('button', { name: 'Subnet' }).click()
62+
await page.getByRole('option', { name: 'mock-subnet' }).click()
63+
64+
const sidebar = page.getByRole('dialog', { name: 'Add network interface' })
65+
66+
// verify that the SideModal header is positioned above the TopBar
67+
await expectObscured(page.getByRole('button', { name: 'User menu' }))
68+
69+
// test that the form can be submitted and a new network interface is created
70+
await sidebar.getByRole('button', { name: 'Add network interface' }).click()
71+
await expect(sidebar).toBeHidden()
72+
await expect(page.getByText('alt-nic')).toBeVisible()
73+
})

libs/ui/lib/date-picker/Popover.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export function Popover(props: PopoverProps) {
3636
<div
3737
{...popoverProps}
3838
ref={ref}
39-
className="rounded-md absolute top-full z-10 mt-2 rounded-lg border bg-raise border-secondary elevation-2"
39+
className="rounded-md absolute top-full z-popover mt-2 rounded-lg border bg-raise border-secondary elevation-2"
4040
>
4141
<DismissButton onDismiss={state.close} />
4242
{children}

libs/ui/lib/listbox/Listbox.tsx

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,22 @@
55
*
66
* Copyright Oxide Computer Company
77
*/
8-
import { FloatingPortal, flip, offset, size, useFloating } from '@floating-ui/react'
8+
import {
9+
FloatingPortal,
10+
autoUpdate,
11+
flip,
12+
offset,
13+
size,
14+
useFloating,
15+
} from '@floating-ui/react'
916
import { Listbox as Select } from '@headlessui/react'
1017
import cn from 'classnames'
1118
import type { ReactNode } from 'react'
1219

13-
import { FieldLabel, SpinnerLoader, TextInputHint } from '@oxide/ui'
14-
import { SelectArrows6Icon } from '@oxide/ui'
20+
import { FieldLabel, SelectArrows6Icon, SpinnerLoader, TextInputHint } from '@oxide/ui'
21+
22+
import { useIsInModal } from '../modal/Modal'
23+
import { useIsInSideModal } from '../side-modal/SideModal'
1524

1625
export type ListboxItem<Value extends string = string> = {
1726
value: Value
@@ -68,11 +77,19 @@ export const Listbox = <Value extends string = string>({
6877
},
6978
}),
7079
],
80+
whileElementsMounted: autoUpdate,
7181
})
7282

7383
const selectedItem = selected && items.find((i) => i.value === selected)
7484
const noItems = !isLoading && items.length === 0
7585
const isDisabled = disabled || noItems
86+
const isInModal = useIsInModal()
87+
const isInSideModal = useIsInSideModal()
88+
const zIndex = isInModal
89+
? 'z-modalDropdown'
90+
: isInSideModal
91+
? 'z-sideModalDropdown'
92+
: 'z-contentDropdown'
7693

7794
return (
7895
<div className={cn('relative', className)}>
@@ -134,7 +151,7 @@ export const Listbox = <Value extends string = string>({
134151
<Select.Options
135152
ref={refs.setFloating}
136153
style={floatingStyles}
137-
className="ox-menu pointer-events-auto z-50 overflow-y-auto !outline-none"
154+
className={`ox-menu pointer-events-auto ${zIndex} overflow-y-auto !outline-none`}
138155
>
139156
{items.map((item) => (
140157
<Select.Option

libs/ui/lib/modal/Modal.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ import { Close12Icon } from '../icons'
1616

1717
const ModalContext = createContext(false)
1818

19-
export const useIsInModal = () => {
20-
return useContext(ModalContext)
21-
}
19+
export const useIsInModal = () => useContext(ModalContext)
2220

2321
export type ModalProps = {
2422
title?: string
@@ -63,7 +61,7 @@ export function Modal({ children, onDismiss, title, isOpen }: ModalProps) {
6361
aria-hidden
6462
/>
6563
<AnimatedDialogContent
66-
className="DialogContent ox-modal pointer-events-auto fixed left-1/2 top-1/2 z-40 m-0 flex max-h-[min(800px,80vh)] w-auto min-w-[28rem] max-w-[32rem] flex-col justify-between rounded-lg border p-0 bg-raise border-secondary elevation-2"
64+
className="DialogContent ox-modal pointer-events-auto fixed left-1/2 top-1/2 z-modal m-0 flex max-h-[min(800px,80vh)] w-auto min-w-[28rem] max-w-[32rem] flex-col justify-between rounded-lg border p-0 bg-raise border-secondary elevation-2"
6765
aria-labelledby={titleId}
6866
style={{
6967
transform: y.to((value) => `translate3d(-50%, ${-50 + value}%, 0px)`),

libs/ui/lib/side-modal/SideModal.tsx

Lines changed: 66 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import * as Dialog from '@radix-ui/react-dialog'
99
import { animated, useTransition } from '@react-spring/web'
1010
import cn from 'classnames'
11-
import React, { type ReactNode, useRef } from 'react'
11+
import React, { type ReactNode, createContext, useContext, useRef } from 'react'
1212

1313
import { Message } from '@oxide/ui'
1414
import { classed } from '@oxide/util'
@@ -18,6 +18,10 @@ import { useIsOverflow } from 'app/hooks'
1818
import { Close12Icon, Error12Icon } from '../icons'
1919
import './side-modal.css'
2020

21+
const SideModalContext = createContext(false)
22+
23+
export const useIsInSideModal = () => useContext(SideModalContext)
24+
2125
export type SideModalProps = {
2226
title?: string
2327
subtitle?: ReactNode
@@ -53,65 +57,69 @@ export function SideModal({
5357
config: isOpen && animate ? config : { duration: 0 },
5458
})
5559

56-
return transitions(
57-
({ x }, item) =>
58-
item && (
59-
<Dialog.Root
60-
open
61-
onOpenChange={(open) => {
62-
if (!open) onDismiss()
63-
}}
64-
// https://github.com/radix-ui/primitives/issues/1159#issuecomment-1559813266
65-
modal={false}
66-
>
67-
<Dialog.Portal>
68-
<div
69-
className="DialogOverlay pointer-events-auto"
70-
onClick={onDismiss}
71-
aria-hidden
72-
/>
73-
<AnimatedDialogContent
74-
className="DialogContent ox-side-modal pointer-events-auto fixed bottom-0 right-0 top-0 m-0 flex w-[32rem] flex-col justify-between border-l p-0 bg-raise border-secondary elevation-2"
75-
aria-labelledby={titleId}
76-
style={{
77-
transform: x.to((value) => `translate3d(${value}%, 0px, 0px)`),
60+
return (
61+
<SideModalContext.Provider value>
62+
{transitions(
63+
({ x }, item) =>
64+
item && (
65+
<Dialog.Root
66+
open
67+
onOpenChange={(open) => {
68+
if (!open) onDismiss()
7869
}}
70+
// https://github.com/radix-ui/primitives/issues/1159#issuecomment-1559813266
71+
modal={false}
7972
>
80-
{title && (
81-
<Dialog.Title asChild>
82-
<>
83-
<SideModal.Title id={titleId} title={title} subtitle={subtitle} />
84-
85-
{errors && errors.length > 0 && (
86-
<div className="mb-6">
87-
<Message
88-
variant="error"
89-
content={
90-
errors.length === 1 ? (
91-
errors[0]
92-
) : (
93-
<>
94-
<div>{errors.length} issues:</div>
95-
<ul className="ml-4 list-disc">
96-
{errors.map((error, idx) => (
97-
<li key={idx}>{error}</li>
98-
))}
99-
</ul>
100-
</>
101-
)
102-
}
103-
title={errors.length > 1 ? 'Errors' : 'Error'}
104-
/>
105-
</div>
106-
)}
107-
</>
108-
</Dialog.Title>
109-
)}
110-
{children}
111-
</AnimatedDialogContent>
112-
</Dialog.Portal>
113-
</Dialog.Root>
114-
)
73+
<Dialog.Portal>
74+
<div
75+
className="DialogOverlay pointer-events-auto"
76+
onClick={onDismiss}
77+
aria-hidden
78+
/>
79+
<AnimatedDialogContent
80+
className="DialogContent ox-side-modal pointer-events-auto fixed bottom-0 right-0 top-0 z-sideModal m-0 flex w-[32rem] flex-col justify-between border-l p-0 bg-raise border-secondary elevation-2"
81+
aria-labelledby={titleId}
82+
style={{
83+
transform: x.to((value) => `translate3d(${value}%, 0px, 0px)`),
84+
}}
85+
>
86+
{title && (
87+
<Dialog.Title asChild>
88+
<>
89+
<SideModal.Title id={titleId} title={title} subtitle={subtitle} />
90+
91+
{errors && errors.length > 0 && (
92+
<div className="mb-6">
93+
<Message
94+
variant="error"
95+
content={
96+
errors.length === 1 ? (
97+
errors[0]
98+
) : (
99+
<>
100+
<div>{errors.length} issues:</div>
101+
<ul className="ml-4 list-disc">
102+
{errors.map((error, idx) => (
103+
<li key={idx}>{error}</li>
104+
))}
105+
</ul>
106+
</>
107+
)
108+
}
109+
title={errors.length > 1 ? 'Errors' : 'Error'}
110+
/>
111+
</div>
112+
)}
113+
</>
114+
</Dialog.Title>
115+
)}
116+
{children}
117+
</AnimatedDialogContent>
118+
</Dialog.Portal>
119+
</Dialog.Root>
120+
)
121+
)}
122+
</SideModalContext.Provider>
115123
)
116124
}
117125

0 commit comments

Comments
 (0)