Add alerts and their macOS implementation.

[xStrom]

API

This PR adds a cross-platform alert API that will enable a good user experience on Windows/macOS/Linux.

The API is designed in a way that a single AlertOptions configuration will result in a native feeling alert on all these platforms. The buttons are ordered automatically, so none of this windows-app-on-mac or vice versa feeling when you encounter an alert with a weird layout.

The API also favors a good user experience with contextualized (and translated) button labels. This is something that all the platform guideline documents agree on. Yes you can find plenty of legacy apps with basic labels like Confirm or Yes but neither of those are actually recommended.

macOS implementation

This PR also contains the macOS implementation of this API to get things started and show that the API works.

I am aware of at least two macOS druid-shell issues that show up in relation to the alerts work. These issues aren't directly related to the alerts API though so I am tackling them separately. The first one is timers not firing when modal dialogs are up, which is resolved in #1028. When that fix is applied another issue surfaces - Application::quit doesn't properly end the runloop when an app modal dialog is up - it just ends the modal runloop. This is an issue I'm still investigating but will also solve separately as it's more of a bug in the quit method.

Alert example

I also added a new alert example. It shows a few different ways to use alerts, although some of it can feel needlessly complex for an example. I built it more as a testing tool to see how edge cases are handled when multiple alerts show up in various ways. This made me think that perhaps we should have an examples directory but also some sort of testing directory of example-like apps that help us confirm whether various implementations are working properly. For now, I think the alert example can live as an example, but this distinction is something to think about for the future.

Future work

I actually started this alerts work with the Windows implementation, which I've had 80% done for months now. This helped me really understand the cross-platform requirements. Moving forward I'll finish up the Windows implementation and post that as a separate PR.

---

Fixes #383.

[justinmoon]

Tested the example on MacOS and everything worked very nicely. Looked very "native".

[ratmice]

When that fix is applied another issue surfaces - Application::quit doesn't properly end the runloop when an app modal dialog is up - it just ends the modal runloop. This is an issue I'm still investigating but will also solve separately as it's more of a bug in the quit method.

Interesting... It looks like this is because the stop method only applies to the current runloop https://github.com/xi-editor/druid/blob/6a429144309cef53c5d8c772b9e9c25416c50cee/druid-shell/src/platform/mac/application.rs#L96

It's not clear to me why this isn't just using terminate:, so perhaps that or calling stop via performSelectorOnMainThread:withObject:waitUntilDone:

[xStrom]

It's not clear to me why this isn't just using terminate:,

Because terminate kills the process and cleanup code won't run.

so perhaps that or calling stop via performSelectorOnMainThread:withObject:waitUntilDone:

That's not enough either. I've found a way to stop two levels of runloops with abortModal + stop, however not three levels yet. I'll try some sort of handle based targeting soon probably.

[cmyr]

This overall looks very good, a few comments inline but really nothing blocking.

I want to do another pass on the docs, but lets get ball rolling.

[cmyr]

I presume that the button is used to resolve what action was taken?

My inclination would be to not use AlertButton for that, and instead have the user pass in some other token for each option? Although now that I write this out I see how annoying it might be.

I guess this comes down to my concern about using string equality to check for intent, I could imagine cases (maybe?) where a locale changes while a modal is up and the user checks against a changed string, or similar?

Perhaps a more reasonable concern is that this doesn't work as well when the buttons are localized, because you wan't have a const AlertButton sitting around.

So I'm not convinced either way, but would be curious to hear your thoughts.

[xStrom]

This is a pain point, I wrestled with various designs and I agree this is not perfect. Right now the idea is that you either have a const AlertButton or you store the localized AlertButton and check against that. This stored one wouldn't change when the locale changes, but it can be a bit of a pain to plumb around because it does not implement Copy.

I was thinking of also having some sort of AlertButtonToken but that runs into the same issue that commands/selectors do, in that this token would have to be a manually specified string to support const. Also having a token would make things less streamlined for const buttons.

Thinking about this some more now, perhaps there is a way to make this somewhat better in a way that wouldn't decrease the ergonomics of const buttons.

We could define that a const button's token is equal to its label. Then AlertResult could have Option<AlertButtonToken> and we could also support AlertButton == AlertButtonToken by using the button's underlying token for equality. To create localized buttons you would have to supply both a token to identify the button, and a label. The AlertButtonToken would implement Copy and be able to be const. The ergonomics win from having such an easily usable token might be greater than the ergonomics loss of having to define the tokens.

[cmyr]

Here's another consideration:

there are special cases, like 'ok' and 'cancel', where in general we want to let the platform provide the strings and handle the localization, (If possible?) and so we'd want to have some nice way of handling this.

some thoughts: we could maybe stick with the current mechanism, but have the "action identifier" or something be different from the display text? And maybe we could also have convenient is_confirm() and is_cancel() methods?

honestly... I defer to you here, you've been thinking about it more, I don't think that from a distance I'm going to come up with some sort of better solution.

[xStrom]

I'll play around a bit more with this API and see if anything seems more ergonomical. The idea of is_cancel is there already in form of None but I guess an additional method to AlertResponse wouldn't hurt much. A similar method for the primary button could also be worth it.

---

I don't think that letting the platform localize some buttons is a good idea. I've used apps like that and it's always a horrible user experience because the platform and app won't have a matching set of translated languages. Solving this is one of the key goals I personally have regarding localization.

If the app doesn't have the language but the platform has, then the alert message (and some buttons!) will be in English while OK/Cancel will be translated. The other way around you could have the message and some buttons translated and then suddenly some English OK/Cancel thrown in to the mix because the platform doesn't have that language. All of this gets even more insane once you mix RTL languages in.

The app should have full localization control. This will allow the user experience in the app to be consistent. If the app doesn't support a locale, then at least everything will be in a locale it supports. On the other hand if it does support a specific locale, then the platform won't be holding it back.

The user should also be able to override the language in the app. Nothing pisses me off more than when an app translates everything into my locale (at Google translate level no less) and has no way to change it. Also in today's globalized world many people have organization provided computers where the language is stuck at whatever the organization has chosen. For example my girlfriend works for a Danish organization and has a Danish language Windows - but she doesn't speak Danish. Not being able to override this choice in apps (or having some buttons suddenly be in Danish) is a very bad experience.

[cmyr]

That makes sense, the only annoyance is that we then want to ship localization files that have the same strings as the platform localization files for every supported locale, which isn't the end of the world.

[luleyleo]

One thing that would come to my mind is separating buttons and tokens, then assigning a (constant) token to each button to check against. This would allow us providing tokens for ok and cancel which will be the most used anyway and if the user has a custom action it would be a single constant to declare, which seems simple enough.

Regarding localization of buttons I agree with @xStrom that it should be entirely under the application's control.

[cmyr]

An additional thought I missed in my original review: I'll try and use this in runebender shortly and see how it feels, that will hopefully give me better perspective.

[luleyleo]

Looks like a good start!
Sadly I can't run any of this, so I can only give theoretic feedback about the code and API.

[luleyleo]

One thing that would come to my mind is separating buttons and tokens, then assigning a (constant) token to each button to check against. This would allow us providing tokens for ok and cancel which will be the most used anyway and if the user has a custom action it would be a single constant to declare, which seems simple enough.

Regarding localization of buttons I agree with @xStrom that it should be entirely under the application's control.

[cmyr]

Just had a thought here: what if you passed a Command along with each alert option? then we could not worry about tokens etc, and just handle the command as usual?

[cmyr]

An update: I implemented a "you have unsaved changes" dialog when trying to quit in runebender, and I'm fairly happy with the result. I do think this idea of just associating a command with each button could be helpful. If we wanted to be very rigorous, we might also want to use a token to identify a specific modal; I'm not sure how necessary this is in practice.

[bjorn3]

nit:

Suggested change

	- Alert dialogs for time sensetive notifications. ([#1039] by [@xStrom])
	- Alert dialogs for time sensitive notifications. ([#1039] by [@xStrom])

[jneem]

What's the status of this? Are there any major changes to the API that need to be made? If not, I'd like to resolve the conflicts and get this merged. (My main limitation is that I can't test on mac, so I don't want to make non-trivial changes to the mac part)

[raphlinus]

Given that the original author has gone radio silent (I hope you're ok, @xStrom, and just taking a break; in any case we appreciate your work on this), I'd be very grateful for you to champion getting this merged, especially as it's a very often requested feature. I'm willing to take on mac testing if nobody else steps up.

[cmyr]

closing this in favor of #1256

[xStrom]

I am now back and will update this PR shortly. I have some API improvement ideas that weren't pushed that I'll also review - and I'll read through the feedback and address that as well.

[arifd]

How is progress on this coming along? Is there anything I can do to help? Basically alerts and PasswordInput are the only two things blocking me from use Druid in real world applications.

[cmyr]

Yea, I need to pick this up and get it finished.