refactor: migrate all NotificationDialog to AlertDialog and remove factories by sven1103 · Pull Request #1489 · qbicsoftware/data-manager-app

sven1103 · 2026-07-27T17:16:09Z

Summary

Replace all NotificationDialog subclasses with AlertDialog patterns and remove the notification factory boilerplate.

Changes

Deleted (12 files)

AccessTokenDeletionConfirmationNotification.java
BatchDeletionConfirmationNotification.java
MeasurementDeletionConfirmationNotification.java
QCItemDeletionConfirmationNotification.java
PurchaseItemDeletionConfirmationNotification.java
ProjectUserRemovalConfirmationNotification.java
ExistingGroupsPreventVariableEdit.java
ExistingSamplesPreventVariableEdit.java
ExistingSamplesPreventSampleOriginEdit.java
ExistingSamplesPreventGroupEdit.java
CancelConfirmationDialogFactory.java
NotificationDialog.java — completely removed (caller must use AlertDialog)

Deleted from MessageSourceNotificationFactory

dialog() method — fully removed (caller inlined as AlertDialog.alert().error())
parseTitle() helper — removed (no longer needed)
parseConfirmText() helper — removed (no longer needed)
DEFAULT_CONFIRM_TEXT constant — removed

Modified call-sites (12 files)

PersonalAccessTokenMain.java — uses AlertDialog.danger()
SampleInformationMain.java — uses AlertDialog.danger() + inlined error dialog
MeasurementMain.java — uses AlertDialog.danger() (3 sites)
ProjectInformationMain.java — uses AlertDialog.danger() + inlined cancel dialog
ProjectAccessComponent.java — uses AlertDialog.danger()
EditExperimentDialog.java — uses AlertDialog.alert().error()
ExperimentDetailsComponent.java — uses AlertDialog.alert().error() + inlined cancel dialog
ProjectMainLayout.java — removed factory parameter
ProjectOverviewMain.java — removed factory parameter
ExperimentMainLayout.java — removed factory parameter
AddProjectDialog.java — removed factory parameter, uses AlertDialog
ProjectSideNavigationComponent.java — removed factory parameter, uses AlertDialog

Documentation

front-end-components.md — added AlertDialog section with class diagram, decision tree, and code examples

Why

Every NotificationDialog subclass was a dead-simple factory with zero reusable logic — just a few lines of boilerplate. AlertDialog handles the dialog lifecycle internally, so callers can write one-liners:

// Before (20+ lines):
var dialog = new BatchDeletionConfirmationNotification();
dialog.open();
dialog.addConfirmListener(event -> { doDelete(); dialog.close(); });
dialog.addCancelListener(event -> dialog.close());

// After (1 line):
AlertDialog.danger(this, "Samples within batch will be deleted",
    "Deleting this Batch will also delete the samples contained within. Proceed?",
    () -> doDelete()).open();

CancelConfirmationDialogFactory was pure boilerplate — identical content everywhere. Callers now use AlertDialog directly.

…ctories Replace all NotificationDialog subclasses with AlertDialog patterns: - Deleted 10 NotificationDialog subclasses (dead-simple factories with zero reusable logic): AccessTokenDeletionConfirmationNotification, BatchDeletionConfirmationNotification, MeasurementDeletionConfirmationNotification, QCItemDeletionConfirmationNotification, PurchaseItemDeletionConfirmationNotification, ProjectUserRemovalConfirmationNotification, ExistingGroupsPreventVariableEdit, ExistingSamplesPreventVariableEdit, ExistingSamplesPreventSampleOriginEdit, ExistingSamplesPreventGroupEdit - Deleted CancelConfirmationDialogFactory — callers now use AlertDialog.alert().warning() directly - Deprecated MessageSourceNotificationFactory.dialog() — single call-site inlined as AlertDialog - Converted NotificationDialog.java to deprecated stub throwing UnsupportedOperationException - Migrated UiExceptionHandler to AlertDialog - Updated front-end-components.md with AlertDialog pattern documentation GitHub Issue: #1487

…og() method - Delete NotificationDialog.java stub entirely - Delete deprecated MessageSourceNotificationFactory.dialog() method and its helper methods (parseTitle, parseConfirmText, DEFAULT_CONFIRM_TEXT) - Remove unused AlertDialog import from MessageSourceNotificationFactory

- WARNING intent now gets red danger button (same as DANGER) - Shortened button labels: 'Discard' instead of 'Discard changes', 'Cancel' instead of 'Continue editing' - Updated AppDialog.createConfirmDialog() for consistency

KochTobi

Hi @sven1103
Very good refactor!!
I have only some minor points. As none of them are critical I already approve. Feel free to have a look regardless

KochTobi · 2026-07-28T12:02:34Z

      // style. If no cancel action is set, the builder skips the cancel
      // button (single-action confirmation).
-      if (intent == Intent.DANGER && cancelLabel != null && cancelAction != null) {
+      if ((intent == Intent.DANGER || intent == Intent.WARNING) && cancelLabel != null && cancelAction != null) {


These if else conditions check if it is confirm only and if not, if the confirm button should be marked as dangerous. As there cannot be a dangerous confirm only dialog (intent DANGER or WARNING and no cancel action), should this be checked explicitly? I think it is very sensible to forbid a dangerous operation where you can only confirm.

KochTobi · 2026-07-28T12:10:32Z

+    AlertDialog.alert(this)
+        .warning()
+        .title("Discard changes?")
+        .message("By aborting the editing process and closing the dialog, you will lose all information entered.")
+        .confirmButton("Discard", () -> editExperimentDialog.close())
+        .cancelButton("Cancel", () -> {})
+        .build()


NIT: This pattern is repeating a lot. The repetition is partly due to the missing adaption of the AppDialog. However, would it make sense to provide a factory method for the cancel confirmation case as long as it is used in multiple locations or would you prefer the eventual refactor to the AppDialog to tackle that at some point? The goal would be that the developer can change the way the cancellation is confirmed centrally somewhere. What do you think?

I am not sure what is missing in the AppDialog? For the factory method, I am unsure if it is a little over engineered for a simple problem. The builder pattern is pretty straight forward and adaptions easy to refactor?

AlertDialog.alert(this) .warning() .title("Discard changes?") .message("By aborting the editing process and closing the dialog, you will lose all information entered.") .confirmButton("Discard", () -> editExperimentDialog.close()) .cancelButton("Cancel", () -> {}) .build()

This code is duplicated across a lot of dialogs right now. Nothing in the AppDialog needs changing. There are some dialogs still using the QbicDialog or similar classes. Thats why you have to wire the cancellation dialog there explicitly. In the AppDialog this is handled centrally. When moving to the AppDialog, the issue is gone. So I think the code is fine as it is. Just wanted to highlight this.

AlertDialog.alert(this) .warning() .title("Discard changes?") .message("By aborting the editing process and closing the dialog, you will lose all information entered.") .confirmButton("Discard", () -> editExperimentDialog.close()) .cancelButton("Cancel", () -> {}) .build()

This code is duplicated across a lot of dialogs right now. Nothing in the AppDialog needs changing. There are some dialogs still using the QbicDialog or similar classes. Thats why you have to wire the cancellation dialog there explicitly. In the AppDialog this is handled centrally. When moving to the AppDialog, the issue is gone. So I think the code is fine as it is. Just wanted to highlight this.

…ented descriptions UX research indicates that AlertDialog confirm/cancel buttons with single-word labels (Confirm, Discard, Cancel, Okay) create unnecessary cognitive load because users must infer the actual outcome from context. This commit improves transparency by making buttons explicitly state their effect: Destructive actions (was: 'Confirm' / 'Cancel'): - 'Remove' / 'Remove user' → 'Keep user' - 'Disconnect' / 'Disconnect dataset' → 'Keep connection' - 'Delete' / 'Delete offer' → 'Keep offer' - 'Delete file' → 'Keep file' - 'Delete batch' → 'Keep batch' - 'Delete measurements' → 'Keep measurements' - 'Delete token' → 'Keep token' Discard-changes confirmation (was: 'Discard' / 'Cancel'): - 'Discard changes' → 'Keep editing' → applied to 6 call sites including internal AppDialog unsaved-changes alert Error acknowledgment (was: 'Okay'): - 'Got it' → applied to 4 error/informational dialogs Every button pair now follows the pattern: both buttons state their outcome. Files changed: - AlertDialog: danger() factory gains cancelLabel parameter - AppDialog: internal createConfirmDialog() uses new labels - 11 call sites updated to use new API - AlertDialogTest: updated for new signature (14 tests, all passing)

…with-alertdialog

KochTobi

Very nice changes!

github-actions Bot added the chore label Jul 27, 2026

sven1103 marked this pull request as ready for review July 27, 2026 17:27

sven1103 requested a review from a team as a code owner July 27, 2026 17:27

sven1103 added 2 commits July 27, 2026 19:38

Apply changes to AppDialog confirmation

7658a61

KochTobi self-requested a review July 28, 2026 11:35

KochTobi previously approved these changes Jul 28, 2026

View reviewed changes

sven1103 linked an issue Jul 28, 2026 that may be closed by this pull request

[Task] Replace NotificationDialog with AlertDialog and Toast notifications to harmonise UI code #1487

Closed

7 tasks

sven1103 dismissed KochTobi’s stale review via 46c046c July 28, 2026 14:20

sven1103 requested a review from KochTobi July 28, 2026 14:20

sven1103 and others added 2 commits July 28, 2026 16:35

Merge branch 'development' into refactor/replace-notification-dialog-…

4ddadc7

…with-alertdialog

Address change requests

0933383

KochTobi approved these changes Jul 29, 2026

View reviewed changes

sven1103 merged commit f30e9e3 into development Jul 29, 2026
3 of 4 checks passed

sven1103 deleted the refactor/replace-notification-dialog-with-alertdialog branch July 29, 2026 06:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: migrate all NotificationDialog to AlertDialog and remove factories - #1489

refactor: migrate all NotificationDialog to AlertDialog and remove factories#1489
sven1103 merged 7 commits into
developmentfrom
refactor/replace-notification-dialog-with-alertdialog

sven1103 commented Jul 27, 2026 •

edited

Loading

Uh oh!

KochTobi left a comment

Uh oh!

Uh oh!

KochTobi Jul 28, 2026

Uh oh!

Uh oh!

Uh oh!

KochTobi Jul 28, 2026

Uh oh!

sven1103 Jul 28, 2026

Uh oh!

KochTobi Jul 29, 2026

Uh oh!

KochTobi Jul 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KochTobi left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sven1103 commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Deleted (12 files)

Deleted from MessageSourceNotificationFactory

Modified call-sites (12 files)

Documentation

Why

Related

Uh oh!

KochTobi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

KochTobi Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

KochTobi Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

sven1103 Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

KochTobi Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

KochTobi Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KochTobi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sven1103 commented Jul 27, 2026 •

edited

Loading