New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework the user filter #1279

Closed
ameshkov opened this Issue Jun 13, 2017 · 12 comments

Comments

Projects
None yet
6 participants
@ameshkov
Member

ameshkov commented Jun 13, 2017

This all started here:
#1255

However, I am not happy with how it works at the moment, so let's rework it properly.

Toolbar menu

adguard for android - user filter moqups 2017-06-13 12-48-23

  • Add rule -- brings up a "New Rule" dialog
  • Remove all -- removes all rules from the user filter. IMPORTANT: we must show a confirmation dialog before actually doing it.
  • Edit filter -- switches user filter to the "edit mode" (multiline text edit)
  • Import -- brings up the "Import" dialog
  • Export -- brings up the "Export" dialog

Empty user filter

Screenshot:

adguard for android - user filter moqups 2017-06-13 12-47-11

New Rule dialog

adguard for android - user filter moqups 2017-06-13 12-51-21

Important notes:

  • Use multiline text input (max rows == 3)
  • Text input font should be monospace

User filter rules list view

Screenshot:

adguard for android - user filter moqups 2017-06-13 12-53-28

Important notes:

  • Use monospace font for list view items
  • Do not allow word-wrapping
  • Depending on the rule text we should use different color for the rules text

Here are the rules for syntax highlighting:

// Depending on the substring we choose class
						{regex: /\!.*/, token: "comment"},
						{regex: /@@.*/,  token: "whitelist"},
						{regex: /%%.*/, token: "js"},
						{regex: /#%#.*/, token: "js"},
						{regex: /#@%#.*/, token: "js"},
						{regex: /#\$#.*/, token: "css-inject"},
						{regex: /#@\$#.*/, token: "css-inject"},
						{regex: /##.*/, token: "css"},
						{regex: /#@#.*/, token: "css"},
						{regex: /\$\$.*/, token: "content"},
						{regex: /\$@\$.*/, token: "content"}

// Rules colors
.cm-s-default .cm-comment {color: #808080;}
.cm-s-default .cm-whitelist {color: #336666;}
.cm-s-default .cm-css {color: #6699cc;}
.cm-s-default .cm-js {color: #999933;}
.cm-s-default .cm-css-inject {color: #4477aa;}
.cm-s-default .cm-content {color: #a176a2;}

Edit rule dialog

Tapping on a list item brings up the "Edit Rule" dialog:
adguard for android - user filter moqups 2017-06-13 12-57-02

It is almost the same as the "New Rule" save for one more option -- "Delete".
IMPORTANT: there should be a confirmation dialog after the user taps "delete".

User filter edit mode

Choosing "Edit filter" in the context menu leads a user to the "user filter edit mode".

Here is how it looks like:

adguard for android - user filter moqups 2017-06-13 12-59-06

Basically, this is just a huge multiline text input with "monospace" font and two options in the context menu: Save and Cancel

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jun 13, 2017

Contributor

#1210 now dupes this, too, though I'd to remind y'all of my request from there:

When in filter-editing mode (either from filtering-log or anyplace else), it'd be great to have additional toggles like $important for $image, $script, &c.

Also, #828, please.

Contributor

TPS commented Jun 13, 2017

#1210 now dupes this, too, though I'd to remind y'all of my request from there:

When in filter-editing mode (either from filtering-log or anyplace else), it'd be great to have additional toggles like $important for $image, $script, &c.

Also, #828, please.

@DrDidi

This comment has been minimized.

Show comment
Hide comment
@DrDidi

DrDidi commented Jun 14, 2017

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 14, 2017

Member

Pls retain the rules order when ex-/importing

Sure, it's done in that alpha build you grabbed in the telegram channel.

Member

ameshkov commented Jun 14, 2017

Pls retain the rules order when ex-/importing

Sure, it's done in that alpha build you grabbed in the telegram channel.

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 14, 2017

Member

When in filter-editing mode (either from filtering-log or anyplace else), it'd be great to have additional toggles like $important for $image, $script, &c.

It will be simply too much for this task. I mean just look at how many different modifiers are out there:
https://kb.adguard.com/en/general/how-to-create-your-own-ad-filters

It should be filed as a separate feature request for sure.

Also, #828, please.

Does "User filter edit mode" already cover it?

Member

ameshkov commented Jun 14, 2017

When in filter-editing mode (either from filtering-log or anyplace else), it'd be great to have additional toggles like $important for $image, $script, &c.

It will be simply too much for this task. I mean just look at how many different modifiers are out there:
https://kb.adguard.com/en/general/how-to-create-your-own-ad-filters

It should be filed as a separate feature request for sure.

Also, #828, please.

Does "User filter edit mode" already cover it?

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 22, 2017

Member

Reworked user filter:
adguard_2.9.124_dev.apk.zip

@vozersky I need you to test it very thoroughly.

What to check:

  1. User filter adding new rule
  2. Removing existing rule
  3. Editing existing rule
  4. Import from a valid file
  5. Import from an invalid file
  6. Search in the user filter
  7. Removing all the rules
  8. Enabling/disabling a rule

Same for the whitelist.

NOTE: Don't forget to check that the changes are actually applied to filtering.

@Revertron the only issue I am aware of at the moment is the problem with action bar icons when search starts: https://monosnap.com/file/gC3u7EXFE4OpnjhcOxO4drcKl3Yo7W

Member

ameshkov commented Jun 22, 2017

Reworked user filter:
adguard_2.9.124_dev.apk.zip

@vozersky I need you to test it very thoroughly.

What to check:

  1. User filter adding new rule
  2. Removing existing rule
  3. Editing existing rule
  4. Import from a valid file
  5. Import from an invalid file
  6. Search in the user filter
  7. Removing all the rules
  8. Enabling/disabling a rule

Same for the whitelist.

NOTE: Don't forget to check that the changes are actually applied to filtering.

@Revertron the only issue I am aware of at the moment is the problem with action bar icons when search starts: https://monosnap.com/file/gC3u7EXFE4OpnjhcOxO4drcKl3Yo7W

@vozersky vozersky self-assigned this Jun 27, 2017

@vozersky

This comment has been minimized.

Show comment
Hide comment
@vozersky

vozersky Jun 27, 2017

Member

@ameshkov

everything works correctly except point 5: Import from an invalid file

STR

  1. Try to import image file as a filter list.

Expected result
Toast Error Message is shown

Actual result

Screenshot

image


Environment

Adguard_2.9.124_dev
Android 7.0
Device: Samsung Galaxy S7

Member

vozersky commented Jun 27, 2017

@ameshkov

everything works correctly except point 5: Import from an invalid file

STR

  1. Try to import image file as a filter list.

Expected result
Toast Error Message is shown

Actual result

Screenshot

image


Environment

Adguard_2.9.124_dev
Android 7.0
Device: Samsung Galaxy S7

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 27, 2017

Member

Should be re-tested when release candidate version is out today

Member

ameshkov commented Jun 27, 2017

Should be re-tested when release candidate version is out today

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 27, 2017

Member

@vozersky please add it to the regular Android test cases.

Member

ameshkov commented Jun 27, 2017

@vozersky please add it to the regular Android test cases.

@ameshkov ameshkov closed this Jun 27, 2017

@vozersky

This comment has been minimized.

Show comment
Hide comment
@vozersky

vozersky Jun 27, 2017

Member

Added

Member

vozersky commented Jun 27, 2017

Added

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jun 30, 2017

Contributor

@AdguardTeam Really excellent work! Thanks!

Contributor

TPS commented Jun 30, 2017

@AdguardTeam Really excellent work! Thanks!

@ilvjyw

This comment has been minimized.

Show comment
Hide comment
@ilvjyw

ilvjyw Jul 3, 2017

Hope to add:
Keep the original rule when importing
@AdguardTeam @ameshkov

ilvjyw commented Jul 3, 2017

Hope to add:
Keep the original rule when importing
@AdguardTeam @ameshkov

@vozersky

This comment has been minimized.

Show comment
Hide comment
@vozersky
Member

vozersky commented Jul 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment