Skip to content

Commit c66dcbb

Browse files
committed
[BNE-107] Documents: size menu on wp link not closes automatically when tapped on ios safari
https://community.openproject.org/wp/BNE-107
1 parent f167e27 commit c66dcbb

10 files changed

Lines changed: 155 additions & 15 deletions

File tree

lib/components/BlockWorkPackage/BlockCards.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ export const BlockCardM = ({
103103
className="op-bn-work-package op-bn-work-package--m"
104104
$inDropdown={inDropdown}
105105
onClick={onClick}
106+
// role=button so iOS treats the card as interactive and fires the click on
107+
// the first tap (a plain div inside the contenteditable needs two). Interim:
108+
// mirrors the inline chip;
109+
role={onClick ? 'button' : undefined}
106110
data-testid="block-card"
107111
style={onClick ? { cursor: 'pointer' } : undefined}
108112
>
@@ -138,6 +142,7 @@ export const BlockCardL = ({
138142
className="op-bn-work-package op-bn-work-package--l"
139143
$inDropdown={inDropdown}
140144
onClick={onClick}
145+
role={onClick ? 'button' : undefined}
141146
data-testid="block-card"
142147
style={onClick ? { cursor: 'pointer' } : undefined}
143148
>
@@ -187,6 +192,7 @@ export const BlockCardXL = ({
187192
className="op-bn-work-package op-bn-work-package--xl"
188193
$inDropdown={inDropdown}
189194
onClick={onClick}
195+
role={onClick ? 'button' : undefined}
190196
data-testid="block-card"
191197
style={onClick ? { cursor: 'pointer' } : undefined}
192198
>

lib/components/BlockWorkPackage/BlockWorkPackage.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ import { defaultWpVariables } from '../WorkPackage/atoms';
1919
import { CHIP_STYLES } from '../WorkPackage/tokens';
2020
import { moveCursorAfterBlock } from '../../utils/cursor';
2121
import { pendingBlockRegistry } from './pendingBlockRegistry';
22+
import { useSuppressFormattingToolbar } from '../../hooks/useSuppressFormattingToolbar';
2223

2324
const Block = styled.div.attrs({ className: 'op-bn-extensions', 'data-testid': 'block-wp-wrapper' })<{ $pending?:boolean; $selected?:boolean }>`
2425
${defaultWpVariables}
2526
background-color: ${({ $pending }) => ($pending ? 'transparent' : 'var(--op-chip-bg)')};
26-
user-select: all;
27+
user-select: none;
28+
-webkit-touch-callout: none;
2729
border-radius: var(--bn-border-radius);
2830
box-shadow: ${({ $selected }) => ($selected ? CHIP_STYLES.focusShadow : 'none')};
2931
${({ $pending }) => $pending && 'position: relative;'}
@@ -68,6 +70,8 @@ export const BlockWorkPackageComponent = ({
6870
const isBlockSelected = selectedBlocks.some((b) => b.id === block.id);
6971
const [isOptionsOpen, setIsOptionsOpen] = useState(false);
7072

73+
useSuppressFormattingToolbar(editor, isOptionsOpen);
74+
7175
const workPackageResult = useWorkPackage(block.props.wpid);
7276
const selectedWorkPackage = workPackageResult.workPackage;
7377

@@ -178,6 +182,7 @@ export const BlockWorkPackageComponent = ({
178182
{!workPackageResult.loading && (workPackageResult.error ?? workPackageResult.unauthorized) && (
179183
<UnavailableCardWrapper
180184
ref={cardRef}
185+
role="button"
181186
onClick={(e) => {
182187
e.stopPropagation();
183188
setIsOptionsOpen((prev) => !prev);

lib/components/InlineWorkPackage/InlineWorkPackageChip.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { useTranslation } from 'react-i18next';
2525
import { defaultWpVariables } from '../WorkPackage/atoms';
2626
import { formatWorkPackageId } from '../../utils/id';
2727
import { useIsNodeInSelection } from '../../hooks/useIsNodeInSelection';
28+
import { useSuppressFormattingToolbar } from '../../hooks/useSuppressFormattingToolbar';
2829
import type { BlockNoteEditor } from '@blocknote/core';
2930

3031
export interface InlineWorkPackageChipProps {
@@ -95,6 +96,8 @@ export const InlineWorkPackageChip = ({ inlineContent, contentRef, editor, updat
9596

9697
const isEditorSelected = useIsNodeInSelection(chipRef, editor);
9798

99+
useSuppressFormattingToolbar(editor, isSelected);
100+
98101
const setRef = (node:HTMLElement | null) => {
99102
chipRef.current = node;
100103
contentRef(node);

lib/components/WorkPackage/OptionsPopover.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
ChevronDownIcon,
1313
} from '@primer/octicons-react';
1414
import {formatWorkPackageId} from '../../utils/id';
15+
import { supportsHover } from '../../utils/device';
1516

1617
export interface WpOptionsProps {
1718
wp?:WorkPackage;
@@ -166,6 +167,7 @@ export const WpOptionsPopover = ({
166167
popoverRef,
167168
placement: 'above',
168169
onClose,
170+
closeOnScroll: supportsHover(),
169171
});
170172

171173
const isBlock = currentSize === undefined;
@@ -179,7 +181,9 @@ export const WpOptionsPopover = ({
179181
};
180182

181183
const content = (
182-
// Prevent editor/parent handlers from stealing focus or closing the popover
184+
// stopPropagation stops the outside-tap handlers from closing the popover.
185+
// Do NOT add preventDefault: on iOS it suppresses the first tap's click, so
186+
// every button then needs a priming tap.
183187
<Popover ref={popoverRef} onMouseDown={(e) => e.stopPropagation()}>
184188
{wp && (
185189
<>
@@ -212,7 +216,7 @@ export const WpOptionsPopover = ({
212216
</PopBtn>
213217

214218
{showSizes && (
215-
<SizeMenu onMouseDown={(e) => e.stopPropagation()}>
219+
<SizeMenu>
216220
<SizeMenuLabel>{t('options.inlineSizeLabel')}</SizeMenuLabel>
217221
{INLINE_SIZE_OPTIONS.map((size) => {
218222
return (

lib/components/WorkPackage/anchoredPopover.tsx

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ export interface AnchoredPopoverOptions {
2222
placement:'above' | 'below';
2323
offset?:number;
2424
onClose:() => void;
25+
// When false, a scroll repositions the popover instead of closing it.
26+
closeOnScroll?:boolean;
2527
}
2628

2729
export const useAnchoredPopover = ({
@@ -30,6 +32,7 @@ export const useAnchoredPopover = ({
3032
placement,
3133
offset = DEFAULT_ANCHOR_OFFSET,
3234
onClose,
35+
closeOnScroll = true,
3336
}:AnchoredPopoverOptions) => {
3437
const [anchorRect, setAnchorRect] = useState<DOMRect | null>(
3538
() => (anchorEl ? getAnchorRect(anchorEl) : null)
@@ -38,16 +41,24 @@ export const useAnchoredPopover = ({
3841
useEffect(() => {
3942
if (!anchorEl) return;
4043
const update = () => setAnchorRect(getAnchorRect(anchorEl));
41-
const handleScroll = () => onClose();
44+
45+
// Coalesce reposition work to one run per frame - scroll fires far faster.
46+
let frame = 0;
47+
const scheduleUpdate = () => {
48+
if (frame) return;
49+
frame = requestAnimationFrame(() => { frame = 0; update(); });
50+
};
51+
const handleScroll = () => (closeOnScroll ? onClose() : scheduleUpdate());
4252

4353
update();
4454
window.addEventListener('scroll', handleScroll, true);
45-
window.addEventListener('resize', update);
55+
window.addEventListener('resize', scheduleUpdate);
4656
return () => {
57+
if (frame) cancelAnimationFrame(frame);
4758
window.removeEventListener('scroll', handleScroll, true);
48-
window.removeEventListener('resize', update);
59+
window.removeEventListener('resize', scheduleUpdate);
4960
};
50-
}, [anchorEl, onClose]);
61+
}, [anchorEl, onClose, closeOnScroll]);
5162

5263
// Position before paint so the popover never flashes at its CSS fallback spot.
5364
useLayoutEffect(() => {
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { useEffect } from 'react';
2+
3+
interface ToolbarStore {
4+
state:boolean;
5+
setState:(value:boolean) => void;
6+
subscribe:(listener:() => void) => () => void;
7+
}
8+
9+
interface ToolbarHost {
10+
getExtension:(key:string) => { store?:ToolbarStore } | undefined;
11+
}
12+
13+
// Keeps BlockNote's built-in formatting toolbar closed while `active`. BlockNote
14+
// re-opens it on any pointerup over a focused editor with a non-empty selection,
15+
// and our chip/block holds a NodeSelection the whole time its popover is open -
16+
// so each tap on a popover button would otherwise stack a second toolbar.
17+
export function useSuppressFormattingToolbar(
18+
editor:ToolbarHost | undefined,
19+
active:boolean,
20+
):void {
21+
useEffect(() => {
22+
if (!active || !editor) return;
23+
const store = editor.getExtension('formattingToolbar')?.store;
24+
if (!store) return;
25+
26+
const enforceClosed = () => {
27+
if (store.state) store.setState(false);
28+
};
29+
enforceClosed();
30+
return store.subscribe(enforceClosed);
31+
}, [editor, active]);
32+
}

lib/hooks/useWorkPackagePreview.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type MouseEvent as ReactMouseEvent,
88
type PointerEvent as ReactPointerEvent,
99
} from 'react';
10+
import { supportsHover } from '../utils/device';
1011

1112
const PREVIEW_OPEN_DELAY = 300;
1213
const PREVIEW_CLOSE_DELAY = 150;
@@ -49,13 +50,7 @@ interface WorkPackagePreview {
4950
// inline work package chip: open/close timing, touch long-press detection, and
5051
// the props to wire onto the chip and the preview card.
5152
export function useWorkPackagePreview({ enabled, suppressed }:UseWorkPackagePreviewOptions):WorkPackagePreview {
52-
const canHover = useMemo(
53-
() =>
54-
typeof window !== 'undefined' &&
55-
typeof window.matchMedia === 'function' &&
56-
window.matchMedia('(hover: hover)').matches,
57-
[]
58-
);
53+
const canHover = useMemo(() => supportsHover(), []);
5954

6055
const [previewOpen, setPreviewOpen] = useState(false);
6156
const openTimer = useRef<number | undefined>(undefined);

lib/utils/device.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// True when the primary pointer can hover (desktop mouse) rather than a touch
2+
// screen like iOS - used to branch touch-specific behaviour.
3+
export function supportsHover():boolean {
4+
return (
5+
typeof window !== 'undefined' &&
6+
typeof window.matchMedia === 'function' &&
7+
window.matchMedia('(hover: hover)').matches
8+
);
9+
}

test/lib/components/integration/editor.blockWorkPackage.browser.test.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,4 +124,26 @@ describe('Block card - remove', () => {
124124
await expect.element(page.getByTestId('block-card')).not.toBeInTheDocument();
125125
await expect.element(page.getByText('Fix login bug')).not.toBeInTheDocument();
126126
});
127+
});
128+
129+
describe('Block card - selection', () => {
130+
it('the card wrapper is not text-selectable', async () => {
131+
renderEditor();
132+
await insertInlineWorkPackageViaSlashMenu();
133+
await convertToCompactCard();
134+
135+
const wrapper = document.querySelector('[data-testid="block-wp-wrapper"]')!;
136+
// user-select: all made a single click highlight the whole card's text.
137+
expect(getComputedStyle(wrapper).userSelect).toBe('none');
138+
});
139+
140+
// role=button is what makes iOS fire the click on the first tap (a plain div
141+
// in the contenteditable needs two); see BlockCards.
142+
it('the clickable card exposes role=button', async () => {
143+
renderEditor();
144+
await insertInlineWorkPackageViaSlashMenu();
145+
await convertToCompactCard();
146+
147+
expect(page.getByTestId('block-card').element().getAttribute('role')).toBe('button');
148+
});
127149
});

test/lib/components/integration/wpOptionsPopover.browser.test.tsx

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,22 @@ import { renderEditor } from '../../../helpers/renderEditor';
77
import {
88
insertInlineWorkPackageViaSlashMenu,
99
openInlineWorkPackagePopover,
10-
convertToCompactCard,
1110
openBlockCardPopover,
11+
convertToCompactCard,
1212
} from '../../../helpers/editorHelpers';
1313

1414
afterEach(() => {
1515
cleanup();
1616
});
1717

18+
// BlockNote leaves an empty display:none toolbar container in the DOM when
19+
// closed, so presence alone is not enough - only a rendered one counts.
20+
const formattingToolbarVisible = () =>
21+
Array.from(document.querySelectorAll('[class*="formatting-toolbar"]')).some((el) => {
22+
const he = el as HTMLElement;
23+
return getComputedStyle(he).display !== 'none' && he.childElementCount > 0;
24+
});
25+
1826
function ChipWrapper({ initialSize, wpid = '123' }:{
1927
initialSize:string;
2028
wpid?:string;
@@ -294,4 +302,49 @@ describe('Inline chip popover UX', () => {
294302
await expect.element(page.getByRole('button', { name: label, exact: true })).toBeVisible();
295303
}
296304
});
305+
});
306+
307+
describe('Options popover coexists with the editor', () => {
308+
it('inline chip: opening the size menu summons no formatting toolbar', async () => {
309+
renderEditor();
310+
await insertInlineWorkPackageViaSlashMenu();
311+
await openInlineWorkPackagePopover();
312+
313+
await userEvent.click(page.getByTitle('Change size'));
314+
315+
await expect.element(page.getByTestId('size-menu')).toBeVisible();
316+
expect(formattingToolbarVisible()).toBe(false);
317+
});
318+
319+
it('inline chip: resizing summons no formatting toolbar', async () => {
320+
renderEditor();
321+
await insertInlineWorkPackageViaSlashMenu();
322+
await openInlineWorkPackagePopover();
323+
await userEvent.click(page.getByTitle('Change size'));
324+
325+
await userEvent.click(page.getByRole('button', { name: 'Compact', exact: true }));
326+
327+
expect(formattingToolbarVisible()).toBe(false);
328+
});
329+
330+
it('inline chip: a scroll closes the popover on desktop', async () => {
331+
renderEditor();
332+
await insertInlineWorkPackageViaSlashMenu();
333+
await openInlineWorkPackagePopover();
334+
335+
window.dispatchEvent(new Event('scroll'));
336+
337+
await expect.element(page.getByTestId('popover-content')).not.toBeInTheDocument();
338+
});
339+
340+
it('block card: opening the popover summons no formatting toolbar', async () => {
341+
renderEditor();
342+
await insertInlineWorkPackageViaSlashMenu();
343+
await convertToCompactCard();
344+
345+
await openBlockCardPopover();
346+
await expect.element(page.getByTestId('popover-content')).toBeVisible();
347+
348+
expect(formattingToolbarVisible()).toBe(false);
349+
});
297350
});

0 commit comments

Comments
 (0)