Opened 22 months ago

Closed 21 months ago

Last modified 20 months ago

#6432 closed defect (fixed)

Regression: Hide remove button for additional filter lists

Reported by: greiner Assignee: saroyanm
Priority: P1 Milestone:
Module: User-Interface Keywords:
Cc: wspee, saroyanm, agiammarchi, jeen, Ross, kzar Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29712664/

Description

Background

While going through the old options page for #6431, I found that we're no longer using the "additional_subscriptions" preference in the new options page.

We expose this preference to administrators so that they can preinstall certain filter lists. It is used for hiding the "remove" button for filter lists which have been added this way.

What to change

Hide any UI related to removing a filter list for those that are listed in the "additional_subscriptions" preference.

See also implementation in old options page.

Attachments (1)

disabled.png (10.0 KB) - added by saroyanm 21 months ago.
New color #b3b3b3

Download all attachments as: .zip

Change History (11)

comment:1 Changed 22 months ago by saroyanm

  • Owner set to saroyanm

comment:2 Changed 22 months ago by saroyanm

  • Cc Ross kzar added
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:3 follow-ups: Changed 22 months ago by jeen

I would suggest the following:

  • For items with a checkbox - grey out the checkbox so that it is inactive.
  • For language filter list items in the table - remove the bin icon and the Change label.
  • For filter list table in the advanced tab - remove bin icon, and disable toggle switch.

comment:4 in reply to: ↑ 3 Changed 22 months ago by saroyanm

Replying to jeen:
Thanks Jeen, just a note:

I would suggest the following:

  • For filter list table in the advanced tab - remove bin icon, and disable toggle switch.

Seem like in old implementation it was still possible to disable/enable the subscription, but not remove it.
I think we should keep toggle button the way it was, but still remove the bin icon and "remove" item from the context menu.

comment:5 Changed 22 months ago by greiner

This ticket is about restoring the expected behavior of the filter lists table. Therefore let's move the discussion about how newly introduced parts of the UI are supposed to behave to spec#154.

comment:6 Changed 21 months ago by jeen

Wasn't aware that you could still enable/ disable the filter list - in that case, you are correct @saroyanm, and let's leave the toggle as it is.

Changed 21 months ago by saroyanm

New color #b3b3b3

comment:7 in reply to: ↑ 3 Changed 21 months ago by saroyanm

Replying to jeen:

I would suggest the following:

  • For items with a checkbox - grey out the checkbox so that it is inactive.

As a result of this, I've created a ticket -> https://gitlab.com/eyeo/specs/spec/issues/155

I'll handle that separately after this issue, after I have a guidance in the gitlab ticket, or updated specs.

comment:8 Changed 21 months ago by abpbot

comment:9 Changed 21 months ago by saroyanm

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:10 Changed 20 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Fixed. Remove buttons for subscriptions added via the additional filter lists setting have been removed.

Chrome 65 / Ubuntu 16.04

Note: See TracTickets for help on using tickets.