Skip to content

ui: replace native window.confirm with custom theme-aware modal - #373

Open
JonanOribe wants to merge 4 commits into
smittix:mainfrom
JonanOribe:feature/custom-modal
Open

JonanOribe wants to merge 4 commits into
smittix:mainfrom
JonanOribe:feature/custom-modal

Conversation

@JonanOribe

Copy link
Copy Markdown
Contributor

The goal of this branch is to unify the styles of the application's modal windows, which currently use the browser's default styling.

The first iteration aimed to:

  • Replace browser-default confirm dialogs with a custom modal to
    ensure consistent look and feel across different browsers (Chrome,
    Firefox, Safari, etc.).
  • Native alert/confirm dialogs break the platform's visual identity,
    cannot be styled, and fail to adapt to the user's selected
    theme (light/night mode).
  • Unifying cross-browser UI components prevents jarring visual
    discrepancies and provides a cohesive, professional user experience
    aligned with the tactical dashboard design.

Before:

image image

Now:

image image

The next step in these iterations will involve identifying all the application's modals to achieve complete unification.

@smittix smittix left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JonanOribe, this looks much better than the browser dialog, and it's good to have it follow the theme. A few things before this becomes the shared confirm for the rest of the app (you mentioned that as the next step):

  1. Make it reusable: showCustomConfirm(title, message, onConfirm) reads as generic, but the button text is fixed to "Yes, Stop All". Could the confirm label be a parameter (default "Confirm")?
  2. Use the theme variables throughout: the light and dark colour sets are almost identical (they already use the CSS variables), so one set would do. The title colours are fixed hex values (#dc2626 / #f87171), and those could be var(--accent-red). The system-ui font could be the app's own font variables so the pop-up matches the rest of the UI.
  3. Keyboard and dismissal: Escape and clicking the dark backdrop should cancel. Focus should move into the pop-up when it opens (ideally to Cancel, since this is a destructive action) and go back to the Kill All button when it closes.
  4. Small one: the closing } of showCustomConfirm has lost its indentation.

None of these block this first step on its own, so it's fine to cover them in your follow-up PR when the other dialogs move across, as long as 1 lands before anything else uses it. Thanks again!

@JonanOribe

Copy link
Copy Markdown
Contributor Author

Already done @smittix

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants