AdguardTeam / PopupBlocker

Popup blocking userscript
GNU Lesser General Public License v3.0
337 stars 31 forks source link

Popup blocker pin button and alert dialog logic #95

Closed ameshkov closed 6 years ago

ameshkov commented 6 years ago

Location

The top-right corner of a web page (https://uploads.adguard.com/up04_p20fx_testcases.adguard.comPopupBlockertest-popup-blocker.html.png)

Alert Lifetime

  1. At first, it is expanded.
  2. Expanded state time is 10 seconds.

    • Alert dialog must not be hidden when the user's mouse cursor is over it.
    • Its lifetime is extended by 2 seconds when the user's mouse cursor leaves it.

    Alert Dialog Options [Edited by @seanl-adg]

    1. Prevent actual selecting of an option: https://uploads.adguard.com/up04_9s1sd_testcases.adguard.comPopupBlockertest-popup-blocker.html.png
    2. Instead, when selecting an option does not have any UI effect, provide a user feedback by showing a toast message like this: https://github.com/AdguardTeam/PopupBlocker/issues/103
    3. After the option is selected, go to point 5.
  3. Then it is collapsed to a pin button.
  4. The pin button lifetime is 10 seconds as well.
    • Pin button must not be hidden when the user's mouse is over it.
    • Its lifetime is extended by 2 seconds when the user's mouse cursor leaves it.
    • Click on a pin button expands (or collapses) the alert dialog (depending on whether it is collapsed or expaneded at the moment).
  5. Then the pin button hides itself.
theseanl commented 6 years ago
  1. Prevent actual selecting of an option: https://uploads.adguard.com/up04_9s1sd_testcases.adguard.comPopupBlockertest-popup-blocker.html.png
  2. Selecting an option should work as if it is a button click.

It is a unusual request, and I don't think it is possible with native HTML <select> tag. We will need to emulate a behavior of <select> element with JS and I need a new markup for this.

ameshkov commented 6 years ago

@seanl-adg what I mean is that you need to catch the select change and react immediately:

  1. Do the necessary action (depending on what was selected)
  2. Change the select back to the default state ("Options")

Is that possible? What needs to be changed in the markup?

theseanl commented 6 years ago
  1. "Allow" and "Don't show" options should lead to a page reload

I'm concerned about this.

  1. It is not clear from the UI message that clicking on buttons will lead to page reload. User may lose data unexpectedly.
  2. Applying changed settings does not need a page refresh, unlike "disable ad blocking".
ameshkov commented 6 years ago

Good point. Instead of reloading the page we need to hide the alert dialog when either "Allow" or "Don't show" is clicked.

theseanl commented 6 years ago

IMO we can also provide some tooltips saying smth like "settings saved!" instead of closing the alert.

theseanl commented 6 years ago

If you changed your mind please update the original issue :)

ameshkov commented 6 years ago

@seanl-adg oh I didn't change, these are two separate things (adding toast notifications and this one).

I mean we can live without toast notifications in the very first release, but we will add them later (or maybe sooner, depends on you:))

theseanl commented 6 years ago

Oh don't get me wrong, check the original issue again. I just to want to tidy up the issue threads :) So do you still think that "Allow" and "Don't show" options should lead to a page reload? :)

ameshkov commented 6 years ago

Nope, just hide the alert dialog (eventually, once toast lib is ready, accompany with a toast notification)

Updated the issue text.

theseanl commented 6 years ago

The top-right corner of a web page (https://uploads.adguard.com/up04_p20fx_testcases.adguard.comPopupBlockertest-popup-blocker.html.png)

The position of the pin and the alert need to be adjusted, I need a new CSS for this.

ameshkov commented 6 years ago

@kaprielov assist with https://github.com/AdguardTeam/PopupBlocker/issues/95#issuecomment-377618991 please

theseanl commented 6 years ago

Implemented in https://github.com/AdguardTeam/PopupBlocker/commit/c77637aac61ab63792df59939fe36b975311cdd8

ameshkov commented 6 years ago

One more thing should be added:

Then it is collapsed to a pin button.

The time-to-live in the collapsed state must be 10 seconds. After that, the pin button gets hidden as well.

vozersky commented 6 years ago

Also, I suggest collapsing the alert window automatically after choosing any option:

image

theseanl commented 6 years ago

Implemented in https://github.com/AdguardTeam/PopupBlocker/commit/1920c3f005e9cfd87d59f49586f298d0abbf5028

theseanl commented 6 years ago

How to test:

  1. When there is no user interaction:

    • trigger a popup
    • observe that an alert dialog is displayed
    • wait 10 sec, and observe that an alert dialog is collapsed
    • wait another 10 sec, and observe that a pin button disappears.
  2. When user hovers mouse cursor over the dialog

    • trigger a popup and an alert dialog
    • (2-1) move the mouse cursor over the dialog several times, and eventually stay on the dialog
    • wait more than 10 sec, observe that the dialog does not collapse
    • move the cursor out of the dialog, wait 2 sec, observe that the dialog collapses
      • Click on the pin button, and go to (2-1), or
    • Move mouse cursor over the pin button several times and eventually stay over it
    • wait 10 sec, observe that the pin button does not disappear
    • move the mouse cursor out of the pin button, wait 2 sec, observe that the pin button disappears
  3. When user clicks on the dialog

    • trigger a popup and an alert dialog
    • click on the dialog, move the cursor out of the dialog
    • wait more than 10 sec, observe that the dialog does not collapse
    • click on the "X" button to collapse the dialog
    • Wait 10 sec, then observe that the pin button disappears
karina-archazh commented 6 years ago

Checked, all of those scenarios workes as expected.