You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

I'm thinking that providing something some sort of priority to the notice could help us determine if we should display the notice immediately or not.

Hi @Ecarrion , I like your suggestion, it solves the general problem being able to immediately display a notice even from a different calling site.

I would not like to over engineer the solution with many priorities. So I propose to define two priorities:

enum NoticePriorty {
    // It will behave as the current implementation does
    case `default`
    // Will cancel the top presented and present immediately
    // If the currently presented has priority immediate 
    // it will be placed after all the `default` priority Notices in the queue
    case immediate 
}

I would add this priority to the Notice since we need to know that is the priority of the currently presented(and in the queue) Notice.

But I see one downside. With the current implementation in this PR you can explicitly cancel a specific notice that you know it is no longer displays valid information.
With the solution with priorities we implicitly assume the importance of an error message over any other message that might be currently displayed, so an important information message might be immediately canceled by an innocuous error notice.

Both solutions overlap, but I think solve slightly different problem. I like the solution with priorities since it is more flexible and solves a more general problem. The downside is trivial since (from what I understand) for important notices the user also receives an email.

I you agree I will move forward with the implementation!

Hi @chrisKns

With the current implementation in this PR you can explicitly cancel a specific notice that you know it is no longer displays valid information.

True, but I wouldn't worry about that. Is not something that we are trying to solve or that it has been a problem so far.

The downside is trivial since (from what I understand) for important notices the user also receives an email.

I'm not sure what you are referring to, but I don't think notices imply an email at all.

I you agree I will move forward with the implementation!

Sure you can proceed.

@Ecarrion I implemented the solution with priorities(not pushed yet), however there seems to be non trivial problems.
Hitting "retry" on a failed notification(caused by not having network connection) it will cause a "Order Successful" Notice to be enqueued and then before the dismiss animation finishes a attempt to enqueue a "Order Fail" message, since this is already displayed it will be ignored:

func enqueue(notice: Notice) {
    guard
        noticeOnScreen != notice, // Ignore if we are already presenting this notice.
        !notices.contains(notice) // Ignore if this notice is already enqueued and waiting for presentation.

That will result in the "Order Successful" being displayed only:

The situation is similar when you try to perform an order status change when a "Order Fail" message is already displayed(from a previous attempt). The optimistic "Order Successful" Notice will be enqueued and then "Order Fail" Notice will try to be enqueued that will fail since it is already displayed. Even if it was enqueued since it had higher priority the last message displayed would be of the "Order Successful".

I think the solution with priorities needs more careful design. I am back to the drawing board. I am hesitant removing the checking if a Notice is already presented because it will have app-wide effect that I can not easily evaluate. We could allow an Notice to be enqueued when an equal is being dismissed, this would solve the first problem(tapping reply or any action) but not the second. Any thoughts would be appreciated!

@Ecarrion You can review the solution with priorities here for reference.
I think the only way to make the solution with priorities work is if either the Notice with immediate priority removes all enqueued Notices, to avoid having any stale/invalid Notices that is in the queue being displayed latter. Of course we can leave the original implementation to allow to explicitly cancel a Notice.

@Ecarrion You can review the solution with priorities here for reference. I think the only way to make the solution with priorities work is if either the Notice with immediate priority removes all enqueued Notices, to avoid having any stale/invalid Notices that is in the queue being displayed latter. Of course we can leave the original implementation to allow to explicitly cancel a Notice.

@chrisKns have you thought/considered something simpler like func enqueue(notice: Notice, inFront: Bool) and make that notice skip the queue, replacing the current notice if there is one presented?

@chrisKns have you thought/considered something simpler like func enqueue(notice: Notice, inFront: Bool) and make that notice skip the queue, replacing the current notice if there is one presented?

Yes and unfortunately I think it will have similar problems. If an "Order Fail" Notice is being presented and before it is auto-dismissed(5 seconds) we make an attempt for another order status change, when the normal optimistic "Order Successful" with be enqueued, and then immediately a second "Order Fail" that will replace the first "Order Fail" on top, after this second "Order Fail" Notice is auto-dismissed the user will see the "Order Successful" that was in the queue. Similarly when you tap Retry on a "Order Fail" Notice, before dismiss animation finishes(i.e. it is still presented) the optimistic "Order Successful" and when a second "Order Fail" Notice will be enqueued.
Clearing the queue when adding the fail Notices will solve the problem but will have other side effects.

Clearing the queue when adding the fail Notices will solve the problem but will have other side effects.

What other side effects?
I'm trying to keep this change at a minimum.
What would happen if we just add a func clearQueue() method that is invoked when we detect a failure happen before enqueuing the failure notice? or maybe func enqueue(notice: Notice, clearQueue: Bool = false)

What other side effects? I'm trying to keep this change at a minimum. What would happen if we just add a func clearQueue() method that is invoked when we detect a failure happen before enqueuing the failure notice? or maybe func enqueue(notice: Notice, clearQueue: Bool = false)

I should elaborate more on the "side effects": since in the queue are added Notices from multiple calling sites, clearing the queue can potentially remove Notices that others have enqueued. Other than that a method func enqueue(notice: Notice, clearQueue: Bool = false) would work fine (keeping also the priorities implementation)

I should elaborate more on the "side effects": since in the queue are added Notices from multiple calling sites, clearing the queue can potentially remove Notices that others have enqueued.

I think that would a "primary" effect 😅

Anyway, as discussed, I will close this PR so we can tackle it later with more time and information.

After trying to solve this issue with different designs here is my experience with the problem and solutions tried:

An initial simplistic implementation that added the ability to explicitly cancel a Notice by adding a func cancel(notice: Notice) to the DefaultNoticePresenter worked but the solution could not be used in other situations where the call site does not have access to the currently displayed notice.

With help in the design suggestions from @Ecarrion I tried other designs that have more flexibility: for example skipping the queue and replacing the currently presented Notice with func enqueue(notice: Notice, inFront: Bool) or adding priorities to the Notices with similar effect(a immediate priority Notice would to first). These work only if the Notice you what to cancel is currently presenting on screen, in case it is enqueued (possibly along with others) there is no way to to replace it without having access to that Notice, other then clearing the entire queue leading to losing notices enqueued from different call sites.

The above scenario is not an edge case, in case for poor network connectivity If an "Order Fail" Notice is being presented and before it is auto-dismissed we make an attempt for another order status change, when the normal optimistic "Order Successful" with be enqueued, and then immediately a second "Order Fail”. Here we want the second "Order Fail” to remove the previous "Order Successful”.
Similar is the case if we tap “Retry” on a "Order Fail" Notice, since the during the dismiss animation the success and fail notices are enqueued.

It seems that these cases you will have to have access to the original Notice` remove only that or empty the entire queue.