Opened 23 months ago

Last modified 4 days ago

#3184 reviewing change

As a user i want to see the whitelist functionality in the safari share dialog

Reported by: sven Assignee: ashephard
Priority: P2 Milestone:
Module: Adblock-Plus-for-iOS Keywords: salsita 2015q4
Cc: mario, jand, vojtab, lisabielik Blocked By:
Blocking: #4791 Platform: iOS
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by mario)

Background

In #3165 we're currently implementing the whitelist functionality into Adblock Plus for iOS. With this functionality you're able to type in URLs into a form field, which will be whitelisted in Safari. There is no possibility to do this setting in Safari itself yet.

What to change

Create an item called "Add to Whitelist" with the Adblock Plus logo placed in the Safari share dialog. After pressed, a dialog should appear with the following structure as shown in this mockup:

UI

  1. Headline: Adblock Plus
  2. Text: The following website will be added to the whitelist.
  3. Website Details:
    1. Website title (stripped indicated by an ... if too long)
    2. Website URL (stripped indicated by an ... if too long)
  4. Text: Content will not be blocked on this website. It may take a few seconds for this setting to take effect.
  5. Left button: Cancel
  6. Right button: Done

Logic

  1. If the left button is clicked, the notification will be closed and the user will be brought back to Safari.
  2. If the left button is clicked, the notification will be closed and the user will be brought back to Safari. Based on #3165, an exception rule is created, which leads to ABP not blocking any more ads on this website. The whitelisted website is added to the whitelist section of the ABP frontend. (See #3165 for more detailed information on how whitelisting is achieved.)

Screenshots

Icons

Attachments (9)

Adblock Plus iOS 19 - whitelist share dialog.png (186.5 KB) - added by sven 23 months ago.
dotted separator.png (17.3 KB) - added by sven 23 months ago.
Adblock Plus logo.png (22.0 KB) - added by sven 23 months ago.
Adblock Plus thumbnail.png (21.1 KB) - added by sven 23 months ago.
Adblock Plus iOS 19 - whitelist dialog.2.png (107.8 KB) - added by sven 23 months ago.
Adblock Plus iOS 19 - whitelist dialog styleguide.png (121.4 KB) - added by sven 23 months ago.
Adblock Plus iOS 19 - whitelist dialog.png (111.6 KB) - added by sven 23 months ago.
211951537.png (66.5 KB) - added by mario 7 months ago.
211951537_2.png (65.0 KB) - added by mario 7 months ago.

Download all attachments as: .zip

Change History (45)

comment:1 Changed 23 months ago by sven

  • Cc mario jand vojtab added
  • Description modified (diff)

comment:2 Changed 23 months ago by mario

  • Cc lisabielik added

Changed 23 months ago by sven

Changed 23 months ago by sven

comment:3 Changed 23 months ago by sven

  • Description modified (diff)

Changed 23 months ago by sven

comment:4 Changed 23 months ago by sven

  • Description modified (diff)

comment:5 Changed 23 months ago by sven

  • Description modified (diff)

comment:6 Changed 23 months ago by sven

  • Description modified (diff)

comment:7 Changed 23 months ago by sven

  • Description modified (diff)

comment:8 Changed 23 months ago by sven

  • Description modified (diff)

comment:9 Changed 23 months ago by philll

"Clicking the "Confirm" button leads you back to the current website and closes the share dialog." I guess that should also make use of the functionality that should have been described in #3165

comment:10 Changed 23 months ago by mario

  • Description modified (diff)

Added more information about what happens when "Confirm" is clicked.

comment:11 Changed 23 months ago by mario

  • Description modified (diff)

comment:12 Changed 23 months ago by mario

  • Keywords salsita added

comment:13 follow-up: Changed 23 months ago by vojtab

1) In the popup modal, there are only Edit and Confirm options. Shouldn't there be an option to cancel as well? Otherwise there is no way to cancel adding url to whitelist (except from actually having to open the content blocker app).

2) Shouldn't there be also an option for removing the site from whitelist, if the current site is already among whitelisted websites? I am, however, not sure if it is easily possible to identify whether the page is in whitelist or not from Safari app extension (Jan will know that).

comment:14 in reply to: ↑ 13 Changed 23 months ago by mario

Replying to vojtab:

1) In the popup modal, there are only Edit and Confirm options. Shouldn't there be an option to cancel as well? Otherwise there is no way to cancel adding url to whitelist (except from actually having to open the content blocker app).

I have to agree - there should indeed be the option to cancel it as well. Unfortunately we don't have the ability to easily revise the style guide of this modal dialog until the end of next week. I'm not familiar with this kind of modal dialogs: Would it be twice the effort to add a cancel button afterwards, as soon as we're ready to provide the style guide?

2) Shouldn't there be also an option for removing the site from whitelist, if the current site is already among whitelisted websites? I am, however, not sure if it is easily possible to identify whether the page is in whitelist or not from Safari app extension (Jan will know that).

As of now we don't want to add too many buttons the share functionality. I can only think of showing a "Remove from Whitelist"-button, that is shown instead of the current "Add to whitelist"-button (as soon as the site is already whitelisted). However, this can be treated as a new feature/issue.
Besides that, there's already the possibility to remove it from the whitelist as soon as #3165 is implemented.

comment:15 follow-up: Changed 23 months ago by vojtab

Thanks Mario. Regarding 1) there is common way how to show multiple buttons on modal in iOS - see example http://nshipster.s3.amazonaws.com/uialertcontroller-alert-one-two-three-cancel.png

comment:16 in reply to: ↑ 15 Changed 23 months ago by mario

Replying to vojtab:

Thanks Mario. Regarding 1) there is common way how to show multiple buttons on modal in iOS - see example http://nshipster.s3.amazonaws.com/uialertcontroller-alert-one-two-three-cancel.png

Using this modal could you still implement the fonts and separator above the buttons as shown in the styleguide?
Would it be twice the effort to stick to the 2 buttons and add a cancel button afterwards?

comment:17 Changed 23 months ago by jand

It is ok to have two buttons for now, and it can be simply extended later.

comment:18 Changed 22 months ago by sven

  • Description modified (diff)

comment:19 Changed 22 months ago by mario

  • Keywords 2015q4 added

comment:20 Changed 22 months ago by lisabielik

The text should read:

"will be added to your Adblock Plus whitelist."
"This means that no content will be blocked on this domain. It may take a few seconds for this setting to take effect."

Last edited 22 months ago by lisabielik (previous) (diff)

comment:21 Changed 22 months ago by mario

  • Priority changed from Unknown to P2

Changed 2015q4-issues to P2

comment:22 Changed 20 months ago by mario

  • Ready set

comment:23 Changed 20 months ago by sven

  • Description modified (diff)

comment:24 Changed 9 months ago by jand

During preparation for review I run into this iOS 10 issue:
The background of the proposed whitelisting dialog is no longer transparent and it is only black. I did not managed to get any working solution. It seems to, that any UI of action extension must cover entire screen.

I come with those two solution:
1) Make the action extension the sharing extension, since both type of extension appears in same type of dialog (sharing extension are listed in upper row). Technically this is not problem, it is only matter of configuration. We just need new coloured images for icons. The problem is just in the logic, that we do not share anything. It might be confusing for client.

2) Make new design for dialog to cover entire screen with possibly more feature. For example, we might editing functionality for whitelisted link, which is currently managed by host application.

comment:25 Changed 9 months ago by mario

  • Ready unset

We'll go for the second option, as making this feature available in the "sharing extension" might confuse users. We'll provide a new screen shot/style guide as soon as possible.

comment:26 Changed 7 months ago by mario

  • Description modified (diff)
  • Ready set

Changed 7 months ago by mario

comment:27 Changed 7 months ago by mario

Ready from our side. Text reviewed by Lisa via InVision

comment:28 Changed 7 months ago by mario

  • Description modified (diff)

comment:29 Changed 7 months ago by pavelz

@mario we found an unsettled issue. Favicon is possible, but technically complex. Martin is OK with leaving it out. This didn't get communicated from Martin/Jan InVision discussion
https://projects.invisionapp.com/d/main#/console/9140728/211951537/comments/76388530

Make a followup story if you wish. But it should not block progressing and closing this archeologic feature.

comment:30 Changed 7 months ago by mario

  • Blocking 4791 added

Changed 7 months ago by mario

comment:31 Changed 7 months ago by mario

  • Description modified (diff)

@pavelz, removed the favicon from the scope of this issue and filed #4791 as a follow up.

comment:32 Changed 7 months ago by jand

@mario: I found another problem with new dialog. In controller with whitelisted websites of ABP application we do not display whitelisted website description. Therefore, it does not make sense to ask for description (or title) in action dialog.

Should I add descriptions to whitelisted websites in host application or should I remove description field in action dialog?

comment:33 Changed 7 months ago by mario

@jand, as discussed in IRC, we'll file a follow-up issue to introduce the title of websites in the host app as well. So we can proceed with this issue.

comment:34 Changed 4 days ago by ashephard

  • Owner set to ashephard

comment:35 Changed 4 days ago by ashephard

  • Status changed from new to reviewing

comment:36 Changed 4 days ago by ashephard

Taken the code from Salsita's repo and placed it on my own local repo in a branch (feature/3184v2). Code is currently in review in Reitveld.

Note: See TracTickets for help on using tickets.