Skip to content

Commit dab0a73

Browse files
committed
Apply EventBoundary to new Gallery "New" menu
Whenever dialogs with textfields are used inside of menus, we have to carefully prevents keyboard events from bubbling otherwise the menu will trigger the selection of the menu item starting with that key rather than allowing the textfield to receive the characters.
1 parent 9d80268 commit dab0a73

File tree

7 files changed

+112
-112
lines changed

7 files changed

+112
-112
lines changed

src/main/webapp/ui/QuirksOfMaterialUi.md

+2-26
Original file line numberDiff line numberDiff line change
@@ -82,30 +82,6 @@ is inside of a `Menu` then any letters that occur as the first character in any
8282
of the `MenuItems` will not be enterable into the `TextField` as the event will
8383
be captured by the `Menu` instead.
8484

85-
The solution is wrap the `Dialog` in a component as defined below that prevents
86-
the key events, and others, from propagating up to the `Menu`.
85+
The solution is wrap the `Dialog` in the EventBoundary component
86+
(src/components/EventBoundary.js).
8787

88-
```
89-
const EventBoundary = ({ children }: { children: Node }) => (
90-
// eslint-disable-next-line
91-
<div
92-
onKeyDown={(e) => {
93-
e.stopPropagation();
94-
}}
95-
onMouseDown={(e) => {
96-
e.stopPropagation();
97-
}}
98-
onClick={(e) => {
99-
e.stopPropagation();
100-
}}
101-
>
102-
{children}
103-
</div>
104-
);
105-
```
106-
107-
The eslint suppression is required because `div`s should not ordinarilly have
108-
event handlers as they cannot be focussed. However, in this case, we are just
109-
using the event listeners on the `div` to prevent the events from further down
110-
the DOM from propagating up; the user need not interact with the `div` itself
111-
for this to work.

src/main/webapp/ui/src/Inventory/components/ContextMenu/ContextMenuAction.js

+1-23
Original file line numberDiff line numberDiff line change
@@ -9,29 +9,7 @@ import ContextMenuSplitButton, {
99
import ListItemIcon from "@mui/material/ListItemIcon";
1010
import ListItemText from "@mui/material/ListItemText";
1111
import { StyledMenuItem } from "../../../components/StyledMenu";
12-
13-
/*
14-
* All of the DOM events that happen inside of a context menu action, such as
15-
* events within a dialog, shouln't propagate outside as the context menu will
16-
* take them to be events that it should respond to by providing keyboard
17-
* navigation. See ../../../../QuirksOfMaterialUi.md, secion
18-
* "Dialogs inside Menus", for more information.
19-
*/
20-
const EventBoundary = ({ children }: { children: Node }) => (
21-
<div
22-
onKeyDown={(e) => {
23-
e.stopPropagation();
24-
}}
25-
onMouseDown={(e) => {
26-
e.stopPropagation();
27-
}}
28-
onClick={(e) => {
29-
e.stopPropagation();
30-
}}
31-
>
32-
{children}
33-
</div>
34-
);
12+
import EventBoundary from "../../../components/EventBoundary";
3513

3614
export type ContextMenuRenderOptions = "button" | "menuitem";
3715

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//@flow
2+
3+
import React from "react";
4+
5+
/**
6+
* All of the DOM events that happen inside of a dialog shouldn't propagate
7+
* outside as the menu will take them to be events that it should respond to by
8+
* providing keyboard navigation. See ../../../../QuirksOfMaterialUi.md, secion
9+
* "Dialogs inside Menus", for more information.
10+
*/
11+
export default ({ children }: { children: Node }) => (
12+
/*
13+
* The eslint suppression is required because `div`s should not ordinarilly
14+
* have event handlers as they cannot be focussed. However, in this case, we
15+
* are just using the event listeners on the `div` to prevent the events from
16+
* further down the DOM from propagating up; the user need not interact with
17+
* the `div` itself for this to work.
18+
*/
19+
20+
// eslint-disable-next-line
21+
<div
22+
onKeyDown={(e) => {
23+
e.stopPropagation();
24+
}}
25+
onMouseDown={(e) => {
26+
e.stopPropagation();
27+
}}
28+
onClick={(e) => {
29+
e.stopPropagation();
30+
}}
31+
>
32+
{children}
33+
</div>
34+
);

src/main/webapp/ui/src/eln-dmp-integration/Argos/ArgosNewMenuItem.js

+10-7
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import NewMenuItem from "../../eln/gallery/components/NewMenuItem";
66
import ArgosIcon from "../../eln/apps/icons/Argos.svg";
77
import { COLOR } from "../../eln/apps/integrations/Argos";
88
import CardMedia from "@mui/material/CardMedia";
9+
import EventBoundary from "../../components/EventBoundary";
910

1011
type ArgosNewMenuItemArgs = {|
1112
onDialogClose: () => void,
@@ -29,13 +30,15 @@ export default function ArgosNewMenuItem({
2930
}}
3031
aria-haspopup="dialog"
3132
/>
32-
<DMPDialog
33-
open={showDMPDialog}
34-
setOpen={(b) => {
35-
setShowDMPDialog(b);
36-
if (!b) onDialogClose();
37-
}}
38-
/>
33+
<EventBoundary>
34+
<DMPDialog
35+
open={showDMPDialog}
36+
setOpen={(b) => {
37+
setShowDMPDialog(b);
38+
if (!b) onDialogClose();
39+
}}
40+
/>
41+
</EventBoundary>
3942
</>
4043
);
4144
}

src/main/webapp/ui/src/eln-dmp-integration/DMPOnline/DMPOnlineNewMenuItem.js

+10-7
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import NewMenuItem from "../../eln/gallery/components/NewMenuItem";
66
import DMPonlineIcon from "../../eln/apps/icons/dmponline.svg";
77
import { COLOR } from "../../eln/apps/integrations/DMPonline";
88
import CardMedia from "@mui/material/CardMedia";
9+
import EventBoundary from "../../components/EventBoundary";
910

1011
type DMPonlineNewMenuItemArgs = {|
1112
onDialogClose: () => void,
@@ -29,13 +30,15 @@ export default function DMPonlineNewMenuItem({
2930
}}
3031
aria-haspopup="dialog"
3132
/>
32-
<DMPDialog
33-
open={showDMPDialog}
34-
setOpen={(b) => {
35-
setShowDMPDialog(b);
36-
onDialogClose();
37-
}}
38-
/>
33+
<EventBoundary>
34+
<DMPDialog
35+
open={showDMPDialog}
36+
setOpen={(b) => {
37+
setShowDMPDialog(b);
38+
onDialogClose();
39+
}}
40+
/>
41+
</EventBoundary>
3942
</>
4043
);
4144
}

src/main/webapp/ui/src/eln-dmp-integration/DMPTool/DMPToolNewMenuItem.js

+10-7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { COLOR } from "../../eln/apps/integrations/DMPTool";
88
import CardMedia from "@mui/material/CardMedia";
99
import axios from "axios";
1010
import { mapNullable } from "../../util/Util";
11+
import EventBoundary from "../../components/EventBoundary";
1112

1213
type DMPToolNewMenuItemArgs = {|
1314
onDialogClose: () => void,
@@ -41,13 +42,15 @@ export default function DMPToolNewMenuItem({
4142
}}
4243
aria-haspopup="dialog"
4344
/>
44-
<DMPDialog
45-
open={showDMPDialog}
46-
setOpen={(b) => {
47-
setShowDMPDialog(b);
48-
onDialogClose();
49-
}}
50-
/>
45+
<EventBoundary>
46+
<DMPDialog
47+
open={showDMPDialog}
48+
setOpen={(b) => {
49+
setShowDMPDialog(b);
50+
onDialogClose();
51+
}}
52+
/>
53+
</EventBoundary>
5154
</>
5255
);
5356
}

src/main/webapp/ui/src/eln/gallery/components/Sidebar.js

+45-42
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import useVerticalRovingTabIndex from "../../../components/useVerticalRovingTabI
5050
import useViewportDimensions from "../../../util/useViewportDimensions";
5151
import { observer } from "mobx-react-lite";
5252
import { autorun } from "mobx";
53+
import EventBoundary from "../../../components/EventBoundary";
5354
library.add(faImage);
5455
library.add(faFilm);
5556
library.add(faFile);
@@ -212,50 +213,52 @@ const NewFolderMenuItem = ({
212213
const { createFolder } = useGalleryActions();
213214
return (
214215
<>
215-
<Dialog
216-
open={open}
217-
onClose={() => {
218-
setOpen(false);
219-
}}
220-
>
221-
<form
222-
onSubmit={(e) => {
223-
e.preventDefault();
224-
void createFolder(path, folderId, name).then(() => {
225-
onDialogClose(true);
226-
});
216+
<EventBoundary>
217+
<Dialog
218+
open={open}
219+
onClose={() => {
220+
setOpen(false);
227221
}}
228222
>
229-
<DialogTitle>New Folder</DialogTitle>
230-
<DialogContent>
231-
<DialogContentText variant="body2" sx={{ mb: 2 }}>
232-
Please give the new folder a name.
233-
</DialogContentText>
234-
<TextField
235-
size="small"
236-
label="Name"
237-
onChange={({ target: { value } }) => setName(value)}
238-
/>
239-
</DialogContent>
240-
<DialogActions>
241-
<Button
242-
onClick={() => {
243-
setName("");
244-
setOpen(false);
245-
onDialogClose(false);
246-
}}
247-
>
248-
Cancel
249-
</Button>
250-
<SubmitSpinnerButton
251-
type="submit"
252-
loading={false}
253-
disabled={false}
254-
label="Create"
255-
/>
256-
</DialogActions>
257-
</form>
258-
</Dialog>
223+
<form
224+
onSubmit={(e) => {
225+
e.preventDefault();
226+
void createFolder(path, folderId, name).then(() => {
227+
onDialogClose(true);
228+
});
229+
}}
230+
>
231+
<DialogTitle>New Folder</DialogTitle>
232+
<DialogContent>
233+
<DialogContentText variant="body2" sx={{ mb: 2 }}>
234+
Please give the new folder a name.
235+
</DialogContentText>
236+
<TextField
237+
size="small"
238+
label="Name"
239+
onChange={({ target: { value } }) => setName(value)}
240+
/>
241+
</DialogContent>
242+
<DialogActions>
243+
<Button
244+
onClick={() => {
245+
setName("");
246+
setOpen(false);
247+
onDialogClose(false);
248+
}}
249+
>
250+
Cancel
251+
</Button>
252+
<SubmitSpinnerButton
253+
type="submit"
254+
loading={false}
255+
disabled={false}
256+
label="Create"
257+
/>
258+
</DialogActions>
259+
</form>
260+
</Dialog>
261+
</EventBoundary>
259262
<NewMenuItem
260263
title="New Folder"
261264
avatar={<CreateNewFolderIcon />}

0 commit comments

Comments
 (0)