Skip to content

Commit

Permalink
lib: Add error message when showing multiple Dialogs
Browse files Browse the repository at this point in the history
With human user/browser interaction there can only ever be one Modal
dialog at a time. However, our tests/CDP driver does not have that
restriction, tests can happily click on main page buttons while a dialog
is open.

This can lead to situations where a test opens a dialog, and then opens
a second one (which will overwrite `WithDialogs.Dialog.dialog` with the
new component) without waiting for the first one to close. Then when the
first dialog handler finally calls `.close()`, it will unexpectedly
close the *second* dialog instead.

These races are a pain to debug [1]. Make them obvious with an error message
which shows both the old and the new dialog element. As our tests now
fail on console.errors(), that will detect/prevent similar race
conditions in our tests.

[1] cockpit-project/cockpit-machines#1292
  • Loading branch information
martinpitt committed Nov 9, 2023
1 parent b18df61 commit 81d98a1
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion pkg/lib/dialogs.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,14 @@ export const WithDialogs = ({ children }) => {
const [dialog, setDialog] = useState(null);

const Dialogs = {
show: setDialog,
show: component => {
if (dialog !== null)
console.error("Dialogs.show() called for",
JSON.stringify(component),
"while a dialog is already open:",
JSON.stringify(dialog));
setDialog(component);
},
close: () => setDialog(null),
isActive: () => dialog !== null
};
Expand Down

0 comments on commit 81d98a1

Please sign in to comment.