-
Notifications
You must be signed in to change notification settings - Fork 277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Standardize messageBox handling on app #1139
Conversation
The method uses showMessageBox under the hood, which is not sync
@tupaschoal I see sometimes we use Yes/No buttons on the footer and at other times big buttons with please and thanks. Maybe we should just use the footer buttons and make the text more neutral? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement 😊
It uses showMessageBoxSync under the hood
be1a792
to
f8e9d95
Compare
\changelog-update |
There were several places using
showMessageBox
andshowMessageBoxSync
throughout the app, but they all called them differently and with different names. This centralizes and standardizes them all so that all dialog interactions feel consistent:Just making the names actually consistent with what they do
Allow people using
showDialogSync
to customize the dialog furtherMake files using
showMessageBox*
use the aliases of window-aux instead.We know this makes mocks work on tests, so let's prepare for the day these are mocked :D
No need to specify titles, they will come from the utility functions themselves.
All dialogs will now have the "Time to Leave" title, with the previous title being a blue title inside the dialog and the previous message further down:
I noticed several dialogs were doing things upon being closed on the X, which should never happen, so I made sure all of them had
cancelId
set to the samedefaultId
which is the action that won't do anything.Made dialogs use the
info
,warning
, ... icons instead of the TTL app icon, which previously felt weirdSmall bug fix
Getting rid of redundant option to "cancel" when the option to say "no" is already there.