FIX: Refine dismiss notification confirmation (PR #15017)

Based on /t/44871 I’ve made some modifications to the dialog mostly for copy and consistency.

Note that I had the choice to use bootbox’s ability to add an icon to a button, but I see that there are zero occurrences of this usage in the codebase, so I made a decision not to introduce something new, and re-introduce the template instead.

GitHub

I am going to be annoying, but you should use the I18n.t helpers in the tests as well, otherwise this test will break if someone updates that text

Looks great, just a minor change.

Same here, I18n.t

Great! I hadn’t noticed that, thanks!

Hmm, thinking about it, shouldn’t the test break if someone updates that text? If the implementation states to use a key that doesn’t exist, it will show up as jibberish and the test won’t catch it.

As spoken, I’ll make the change for consistency!

Do we have many/any other alerts that don’t say what’s the operation you’re accepting/dismissing?

@CvX what do you mean? This alert only comes up after pressing the Dismiss Notifications button when you have > 1 green (important) notifications. I think it’s pretty clear what you are accepting with this modal, and the copy was discussed on the internal topic too.

I think it’s pretty clear what you are accepting with this modal

It often is pretty clear, but we still reiterate (in all “are you sure” modals) what the action is.

I wanna prevent more bikeshedding, so have a look /t/44871

The title of this pull request changed from “Refine dismiss notification confirmation” to "FIX: Refine dismiss notification confirmation