fix(opds): show Add Catalog dialog above Settings on mobile (#4669)
The "Add OPDS Catalog" dialog (a ModalPortal opened from inside Settings > Integrations > OPDS Catalogs) rendered behind the Settings sheet on mobile, so the form could not be reached or filled in. Root cause: PR #3235 raised the Settings dialog to z-[10050] to clear the full-screen RSVP overlay (z-[10000]) for in-overlay dictionary management. That also jumped Settings above the ModalPortal layer (z-[100]), so any modal opened from inside Settings was buried. The bug is mobile-only because on desktop the rounded-window frame (.window-border, z-99) traps the inline-rendered Settings dialog in its own stacking context, while ModalPortal escapes to document.body and wins there. Redesign the overlay z-index into a compact scale (no four-digit values), each layer clearing the z-99 page frame: 100 RSVP overlay 101 RSVP controls (start dialog, lookup chip) 110 Settings dialog 120 modal / command palette 130 toast / alert 200 app lock Lock the ordering with a static test that reads the values from source and would have caught the #3235 regression. Documented in DESIGN.md. Verified on a Xiaomi device via CDP: elementFromPoint at the dialog center now resolves inside the Add Catalog form instead of Settings. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
@@ -628,6 +628,29 @@ the closest analog.
|
||||
- Drag handle at top (the small horizontal pill) is mandatory if the sheet supports
|
||||
swipe-to-dismiss.
|
||||
|
||||
#### Stacking order (z-index scale)
|
||||
|
||||
Full-screen and body-portaled overlays share **one global stacking scale**. Keep it
|
||||
compact — never reach for four-digit z-indexes. Every layer must clear the desktop
|
||||
rounded-window page frame (`.window-border`, `z-99` in `globals.css`), then layer:
|
||||
|
||||
| z-index | Layer | Where |
|
||||
| ------- | ----- | ----- |
|
||||
| `99` | Desktop window-border page frame | `globals.css` |
|
||||
| `100` | RSVP immersive reading overlay | `RSVPOverlay` |
|
||||
| `101` | RSVP immersive controls (start dialog, lookup chip) | `RSVPStartDialog`, `RSVPOverlay` |
|
||||
| `110` | Settings app dialog (above RSVP for in-overlay dictionary mgmt) | `SettingsDialog` |
|
||||
| `120` | Modal / command palette | `ModalPortal`, `CommandPalette` |
|
||||
| `130` | Toast / alert | `Alert` |
|
||||
| `200` | Security lock screen | `AppLockScreen` |
|
||||
|
||||
The non-obvious invariant: **`ModalPortal` (120) must stay above `SettingsDialog`
|
||||
(110)** so a modal opened _from inside_ Settings (e.g. Add OPDS Catalog) isn't buried.
|
||||
This bites only on mobile — desktop traps `SettingsDialog` inside the `z-99`
|
||||
`.window-border` stacking context, so the body-portaled modal already wins there.
|
||||
The ordering is locked by `src/__tests__/styles/zIndexScale.test.ts`; update both
|
||||
together.
|
||||
|
||||
---
|
||||
|
||||
### 7. Motion + a11y
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { describe, it, expect } from 'vitest';
|
||||
|
||||
/**
|
||||
* Coordinated overlay z-index scale.
|
||||
*
|
||||
* Body-portaled / full-screen overlays share a single global stacking order.
|
||||
* They must all clear the desktop rounded-window page frame (`.window-border`,
|
||||
* z-index 99 in globals.css) and then layer in this order (low -> high):
|
||||
*
|
||||
* 100 RSVP immersive reading overlay
|
||||
* 101 RSVP immersive controls (start dialog / hint chip)
|
||||
* 110 Settings app dialog (raised above RSVP for dictionary management)
|
||||
* 120 modal / command-palette layer (ModalPortal, CommandPalette)
|
||||
* 130 toast / alert
|
||||
* 200 security lock screen (AppLockScreen)
|
||||
*
|
||||
* This test reads the values straight from source so a future change that
|
||||
* re-orders the layers fails loudly. It encodes the regression that buried the
|
||||
* "Add OPDS Catalog" dialog behind Settings on mobile: Settings had been raised
|
||||
* to z-[10050] (PR #3235) above the ModalPortal at z-[100], so any modal opened
|
||||
* from inside Settings rendered behind it.
|
||||
*/
|
||||
|
||||
const PAGE_FRAME = 99; // .window-border in src/styles/globals.css
|
||||
|
||||
const read = (rel: string) => fs.readFileSync(path.join(process.cwd(), rel), 'utf8');
|
||||
const firstZ = (src: string, re: RegExp): number => {
|
||||
const m = src.match(re);
|
||||
expect(m, `expected to find z-index via ${re} in source`).not.toBeNull();
|
||||
return Number(m![1]);
|
||||
};
|
||||
|
||||
const MODAL = firstZ(read('src/components/ModalPortal.tsx'), /z-\[(\d+)\]/);
|
||||
const SETTINGS = firstZ(read('src/components/settings/SettingsDialog.tsx'), /!z-\[(\d+)\]/);
|
||||
const RSVP_OVERLAY = firstZ(
|
||||
read('src/app/reader/components/rsvp/RSVPOverlay.tsx'),
|
||||
/fixed inset-0 z-\[(\d+)\] flex select-none/,
|
||||
);
|
||||
const RSVP_CONTROLS = firstZ(
|
||||
read('src/app/reader/components/rsvp/RSVPStartDialog.tsx'),
|
||||
/z-\[(\d+)\]/,
|
||||
);
|
||||
const APP_LOCK = firstZ(read('src/components/AppLockScreen.tsx'), /z-\[(\d+)\]/);
|
||||
|
||||
describe('overlay z-index scale', () => {
|
||||
it('renders a modal (e.g. Add OPDS Catalog) above the Settings dialog', () => {
|
||||
// Regression: ModalPortal opened from inside Settings was buried (#add-catalog).
|
||||
expect(MODAL).toBeGreaterThan(SETTINGS);
|
||||
});
|
||||
|
||||
it('raises the Settings dialog above the RSVP immersive overlay', () => {
|
||||
expect(SETTINGS).toBeGreaterThan(RSVP_OVERLAY);
|
||||
});
|
||||
|
||||
it('keeps RSVP controls above the RSVP overlay', () => {
|
||||
expect(RSVP_CONTROLS).toBeGreaterThan(RSVP_OVERLAY);
|
||||
});
|
||||
|
||||
it('keeps the RSVP overlay above the desktop window-border page frame', () => {
|
||||
expect(RSVP_OVERLAY).toBeGreaterThan(PAGE_FRAME);
|
||||
});
|
||||
|
||||
it('keeps the security lock screen on top of every modal', () => {
|
||||
expect(APP_LOCK).toBeGreaterThan(MODAL);
|
||||
});
|
||||
|
||||
it('uses a compact scale with no four-digit z-index', () => {
|
||||
for (const value of [RSVP_OVERLAY, RSVP_CONTROLS, SETTINGS, MODAL, APP_LOCK]) {
|
||||
expect(value).toBeLessThan(1000);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1031,7 +1031,7 @@ const RSVPControl = forwardRef<RSVPControlHandle, RSVPControlProps>(function RSV
|
||||
const dictionaryLang = bookData?.bookDoc?.metadata?.language as string | undefined;
|
||||
const handleManageDictionary = useCallback(() => {
|
||||
// Open dictionary management OVER the RSVP overlay (RSVP stays open). The
|
||||
// settings dialog is raised above the overlay's z-[10000] (see SettingsDialog),
|
||||
// settings dialog is raised above the overlay's z-[100] (see SettingsDialog),
|
||||
// and RSVP's capture-phase keyboard handler bails while it's open so the
|
||||
// settings inputs / Escape work (see RSVPOverlay).
|
||||
setSettingsDialogBookKey(bookKey);
|
||||
|
||||
@@ -685,7 +685,7 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
<div
|
||||
data-testid='rsvp-overlay'
|
||||
aria-label={_('Speed Reading')}
|
||||
className='fixed inset-0 z-[10000] flex select-none flex-col'
|
||||
className='fixed inset-0 z-[100] flex select-none flex-col'
|
||||
style={{
|
||||
paddingTop: `${gridInsets.top}px`,
|
||||
paddingBottom: `${gridInsets.bottom * 0.33}px`,
|
||||
@@ -1323,7 +1323,7 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
{lookup && (
|
||||
<button
|
||||
aria-label={_('Look up')}
|
||||
className='eink-bordered fixed z-[10001] flex -translate-x-1/2 -translate-y-full items-center gap-1.5 rounded-full px-3 py-1.5 text-sm font-semibold shadow-lg'
|
||||
className='eink-bordered fixed z-[101] flex -translate-x-1/2 -translate-y-full items-center gap-1.5 rounded-full px-3 py-1.5 text-sm font-semibold shadow-lg'
|
||||
style={{
|
||||
left: `${lookup.left}px`,
|
||||
top: `${lookup.top}px`,
|
||||
|
||||
@@ -26,7 +26,7 @@ const RSVPStartDialog: React.FC<RSVPStartDialogProps> = ({ startChoice, onSelect
|
||||
return (
|
||||
<div
|
||||
role='presentation'
|
||||
className='fixed inset-0 z-[10001] flex items-center justify-center'
|
||||
className='fixed inset-0 z-[101] flex items-center justify-center'
|
||||
style={{ backgroundColor: backdropColor }}
|
||||
onClick={onClose}
|
||||
onKeyDown={(e) => e.key === 'Escape' && onClose()}
|
||||
|
||||
@@ -14,7 +14,7 @@ const Alert: React.FC<{
|
||||
const divRef = useKeyDownActions({ onCancel, onConfirm });
|
||||
|
||||
return (
|
||||
<div className={clsx('z-[100] flex justify-center px-4')}>
|
||||
<div className={clsx('z-[130] flex justify-center px-4')}>
|
||||
<div
|
||||
ref={divRef}
|
||||
role='alert'
|
||||
|
||||
@@ -7,11 +7,18 @@ interface ModalPortalProps {
|
||||
showOverlay?: boolean;
|
||||
}
|
||||
|
||||
// Coordinated overlay z-index scale (all clear the desktop `.window-border`
|
||||
// page frame at z-99 — see globals.css). Low -> high:
|
||||
// 100 RSVP overlay · 101 RSVP controls · 110 Settings dialog ·
|
||||
// 120 modal / command palette · 130 toast · 200 app-lock.
|
||||
// ModalPortal is the top modal layer, so it sits above the Settings dialog —
|
||||
// a modal opened from inside Settings (e.g. Add OPDS Catalog) must win.
|
||||
// Invariants are enforced by src/__tests__/styles/zIndexScale.test.ts.
|
||||
const ModalPortal: React.FC<ModalPortalProps> = ({ children, showOverlay = true }) => {
|
||||
return ReactDOM.createPortal(
|
||||
<div
|
||||
className={clsx(
|
||||
'fixed inset-0 isolate z-[100] flex items-center justify-center',
|
||||
'fixed inset-0 isolate z-[120] flex items-center justify-center',
|
||||
showOverlay && 'bg-black bg-opacity-50',
|
||||
)}
|
||||
style={{ transform: 'translateZ(0)' }}
|
||||
|
||||
@@ -126,7 +126,7 @@ const CommandPalette: React.FC = () => {
|
||||
return (
|
||||
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
|
||||
<div
|
||||
className='fixed inset-0 z-[100] flex items-start justify-center bg-black/50 pt-[15vh]'
|
||||
className='fixed inset-0 z-[120] flex items-start justify-center bg-black/50 pt-[15vh]'
|
||||
onClick={handleBackdropClick}
|
||||
>
|
||||
{/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */}
|
||||
|
||||
@@ -347,11 +347,12 @@ const SettingsDialog: React.FC<{ bookKey: string }> = ({ bookKey }) => {
|
||||
<Dialog
|
||||
isOpen={true}
|
||||
onClose={handleClose}
|
||||
// Settings is the top-level modal: raise it above the full-screen RSVP
|
||||
// speed-reader overlay (z-[10000]) so dictionary management opened from
|
||||
// inside RSVP shows on top instead of behind it (#3235). !important beats
|
||||
// the Dialog's hardcoded z-50.
|
||||
className='modal-open !z-[10050]'
|
||||
// Settings sits in the overlay z-index scale (see ModalPortal.tsx) above
|
||||
// the RSVP immersive overlay (z-100) so dictionary management opened from
|
||||
// inside RSVP shows on top instead of behind it (#3235), and below the
|
||||
// modal layer (z-120) so a modal opened from inside Settings (e.g. Add
|
||||
// OPDS Catalog) renders on top. !important beats the Dialog's hardcoded z-50.
|
||||
className='modal-open !z-[110]'
|
||||
bgClassName={bookKey ? 'sm:!bg-black/20' : 'sm:!bg-black/50'}
|
||||
boxClassName={clsx(
|
||||
'sm:min-w-[520px] overflow-hidden not-eink:bg-base-200',
|
||||
|
||||
Reference in New Issue
Block a user