From 9c91eb533ddac77d8f254dd53b886e54bb6c787a Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Thu, 9 Nov 2023 11:41:31 +0100 Subject: [PATCH] lib: Add error message when showing multiple Dialogs 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] https://github.com/cockpit-project/cockpit-machines/issues/1292 --- pkg/lib/dialogs.jsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/lib/dialogs.jsx b/pkg/lib/dialogs.jsx index a1e5568b1b7c..4c08c8fdd710 100644 --- a/pkg/lib/dialogs.jsx +++ b/pkg/lib/dialogs.jsx @@ -106,7 +106,14 @@ export const WithDialogs = ({ children }) => { const [dialog, setDialog] = useState(null); const Dialogs = { - show: setDialog, + show: component => { + if (component && 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 };