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

Implement Filtered Apps settings section #633

Closed
ameshkov opened this Issue Jun 10, 2016 · 29 comments

Comments

Projects
None yet
3 participants
@ameshkov
Member

ameshkov commented Jun 10, 2016

1.) No more firewall Apps & Settings
2.) Instead of this section we add a new section named "Filtered Apps"

Layout is almost similar to the Firewall Apps and Settings, except some things:

  1. Get rid of "global" buttons. These buttons are misleading.
  2. Change buttons style on app details view. Use standard "switch style" buttons instead of our custom image buttons.

App settings
Filter Traffic [x]

(if this switch is disabled, gray out all other switches)

Firewall Options:
Mobile Data [x]
Wi-Fi [x]

Ad Blocking Options
Block Ads [x]
Filter HTTPS [x]

@ameshkov ameshkov added the UI label Jun 10, 2016

@ameshkov ameshkov added this to the 2.7 milestone Jun 10, 2016

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 28, 2016

Member

First mockups:
2016-06-28_1445
2016-06-28_1446

Member

ameshkov commented Jun 28, 2016

First mockups:
2016-06-28_1445
2016-06-28_1446

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jun 28, 2016

Contributor

These look fine! I'm hoping, in the 1st pic, that the right-hand column icons (AG shield, wifi antenna, 📶) are quick toggles, & not just static state notifications.

Contributor

TPS commented Jun 28, 2016

These look fine! I'm hoping, in the 1st pic, that the right-hand column icons (AG shield, wifi antenna, 📶) are quick toggles, & not just static state notifications.

@Revertron

This comment has been minimized.

Show comment
Hide comment
@Revertron

Revertron Jun 28, 2016

Member

@TPS No, those are indicators, not buttons.

Member

Revertron commented Jun 28, 2016

@TPS No, those are indicators, not buttons.

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jun 28, 2016

Contributor

Is it practical to make them buttons?

Contributor

TPS commented Jun 28, 2016

Is it practical to make them buttons?

@Revertron

This comment has been minimized.

Show comment
Hide comment
@Revertron

Revertron Jun 28, 2016

Member

They will be smaller, and there will be plenty of mis-clicks.

Member

Revertron commented Jun 28, 2016

They will be smaller, and there will be plenty of mis-clicks.

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jun 28, 2016

Contributor

I understand, but it would be massively more convenient if it could be worked out, especially if 1 needs to change en masse a # of apps at once.

Contributor

TPS commented Jun 28, 2016

I understand, but it would be massively more convenient if it could be worked out, especially if 1 needs to change en masse a # of apps at once.

@Revertron

This comment has been minimized.

Show comment
Hide comment
@Revertron

Revertron Jun 29, 2016

Member

@TPS I will think about it :)

Member

Revertron commented Jun 29, 2016

@TPS I will think about it :)

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 29, 2016

Member

@TPS we aren't re-designing an app yet, so too early for this.

Member

ameshkov commented Jun 29, 2016

@TPS we aren't re-designing an app yet, so too early for this.

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jun 29, 2016

Contributor

we aren't re-designing an app yet, so too early for this.

@ameshkov 😕 This is due for v2.7, right?

Contributor

TPS commented Jun 29, 2016

we aren't re-designing an app yet, so too early for this.

@ameshkov 😕 This is due for v2.7, right?

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 29, 2016

Member

@TPS, yep, this update is more about features, not the design

Member

ameshkov commented Jun 29, 2016

@TPS, yep, this update is more about features, not the design

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jun 29, 2016

Contributor

💡 Ahh, I missed the difference between this & #211! ☺

Contributor

TPS commented Jun 29, 2016

💡 Ahh, I missed the difference between this & #211! ☺

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jun 30, 2016

Member

At the bottom of app details we should have a section with technical information about the app.

Here is what we need to see there:
package name
uid
app type (system or not system)
version
target sdk version
min sdk version
app permissions

Tapping on this block puts all the text to the clipboard (with a toast message)

Member

ameshkov commented Jun 30, 2016

At the bottom of app details we should have a section with technical information about the app.

Here is what we need to see there:
package name
uid
app type (system or not system)
version
target sdk version
min sdk version
app permissions

Tapping on this block puts all the text to the clipboard (with a toast message)

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 5, 2016

Member

We should also add "Reset settings" button to Firewall settings.

Member

ameshkov commented Jul 5, 2016

We should also add "Reset settings" button to Firewall settings.

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 5, 2016

Member

"Access blocked" notification should lead to the app details screen

Member

ameshkov commented Jul 5, 2016

"Access blocked" notification should lead to the app details screen

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 5, 2016

Member

Add "roaming" field to the database table (for future)

Member

ameshkov commented Jul 5, 2016

Add "roaming" field to the database table (for future)

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 6, 2016

Member

Reminder: blocking mobile data when screen is off does not work properly in the current dev build, we should test it more.

Member

ameshkov commented Jul 6, 2016

Reminder: blocking mobile data when screen is off does not work properly in the current dev build, we should test it more.

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 7, 2016

Member

Reminder: check the list of excluded apps

Member

ameshkov commented Jul 7, 2016

Reminder: check the list of excluded apps

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 7, 2016

Member

@Revertron Tech info for "Android OS" app shows an error. We should not show this block at all.

Member

ameshkov commented Jul 7, 2016

@Revertron Tech info for "Android OS" app shows an error. We should not show this block at all.

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 7, 2016

Member

@Revertron maybe it would be better if for "Android OS" we will have a simple description inside the Tech info". Something like that: "Meta package that represents system processes".

Member

ameshkov commented Jul 7, 2016

@Revertron maybe it would be better if for "Android OS" we will have a simple description inside the Tech info". Something like that: "Meta package that represents system processes".

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 7, 2016

Member

@Revertron wrong activity title, should be "Apps and Statistics", not "Firewall"

Member

ameshkov commented Jul 7, 2016

@Revertron wrong activity title, should be "Apps and Statistics", not "Firewall"

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 13, 2016

Member

Done

Member

ameshkov commented Jul 13, 2016

Done

@ameshkov ameshkov closed this Jul 13, 2016

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jul 15, 2016

Contributor

Just got 2.7.151ß (Amazon). Very nice work, folks! Very clean & informative! Just a couple of new minor suggestions/nits:

Right now, the panel is setup with some "implied" kind of UI relationships, e.g., if ad-filtering is disabled for a particular app, that immediately disables/grays out the HTTPS-filtering option for that app.

  • I propose to extend these "implied" relationships further to the converse, e.g., attempting to enable HTTPS-filtering for an app where ad-filtering is disabled shouldn't fail — it should enable both HTTPS- & ad-filtering, to make clear that requirement.
    Further per-app relationships should be:
    • WiFi ↔ Wifi when screen off
    • Cellular data ↔ Cellular data when screen off
    • Firewall notifications when app traffic is routed through AG, but (inexplicably) no other notifiable options are enabled
  • Global ↔ per-app relationships should be shown a bit clearer, e.g., when HTTPS-filtering isn't enabled @ all, the per app settings shouldn't be disabled, but grayed out (potential to keep previous setting) to show it's being ignored. Attempting to enable these per-app shouldn't immediately enable the global setting, but put ↑ a box asking, "What do you mean? Turn on [e.g., HTTPS-filtering] for all apps Leave previous per-app settings Cancel/Change nothing" Further relationships of this sort are the global ↔ per-app settings for wifi, cellular data, firewall itself, & app-traffic filtering.
  • When doing such an ignoring/gray-out, it'd be helpful to put a note w/ the gray-out explaining which setting is causing the ignoring, e.g., "Global HTTPS-filtering is disabled" or "Wi-Fi is disabled for this app."
Contributor

TPS commented Jul 15, 2016

Just got 2.7.151ß (Amazon). Very nice work, folks! Very clean & informative! Just a couple of new minor suggestions/nits:

Right now, the panel is setup with some "implied" kind of UI relationships, e.g., if ad-filtering is disabled for a particular app, that immediately disables/grays out the HTTPS-filtering option for that app.

  • I propose to extend these "implied" relationships further to the converse, e.g., attempting to enable HTTPS-filtering for an app where ad-filtering is disabled shouldn't fail — it should enable both HTTPS- & ad-filtering, to make clear that requirement.
    Further per-app relationships should be:
    • WiFi ↔ Wifi when screen off
    • Cellular data ↔ Cellular data when screen off
    • Firewall notifications when app traffic is routed through AG, but (inexplicably) no other notifiable options are enabled
  • Global ↔ per-app relationships should be shown a bit clearer, e.g., when HTTPS-filtering isn't enabled @ all, the per app settings shouldn't be disabled, but grayed out (potential to keep previous setting) to show it's being ignored. Attempting to enable these per-app shouldn't immediately enable the global setting, but put ↑ a box asking, "What do you mean? Turn on [e.g., HTTPS-filtering] for all apps Leave previous per-app settings Cancel/Change nothing" Further relationships of this sort are the global ↔ per-app settings for wifi, cellular data, firewall itself, & app-traffic filtering.
  • When doing such an ignoring/gray-out, it'd be helpful to put a note w/ the gray-out explaining which setting is causing the ignoring, e.g., "Global HTTPS-filtering is disabled" or "Wi-Fi is disabled for this app."
@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jul 17, 2016

Contributor

Also, I forgot to mention: AG itself still can be blocked from Wifi, or enabled for ad-blocking, &c., @ least according to the UI.… Some part of that should be permanently gray-out with some sort of default setting, right?

Contributor

TPS commented Jul 17, 2016

Also, I forgot to mention: AG itself still can be blocked from Wifi, or enabled for ad-blocking, &c., @ least according to the UI.… Some part of that should be permanently gray-out with some sort of default setting, right?

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 18, 2016

Member

attempting to enable HTTPS-filtering for an app where ad-filtering is disabled shouldn't fail — it should enable both HTTPS- & ad-filtering, to make clear that requirement

It will look strange if we make grayed out item usable.

WiFi ↔ Wifi when screen off
Cellular data ↔ Cellular data when screen off

Yep, this should be done:
#697

Firewall notifications when app traffic is routed through AG, but (inexplicably) no other notifiable options are enabled

Didn't get that point, what do you mean?

when HTTPS-filtering isn't enabled @ all, the per app settings shouldn't be disabled, but grayed out (potential to keep previous setting) to show it's being ignored

#698

Attempting to enable these per-app shouldn't immediately enable the global setting, but put ↑ a box asking

I guess this is about HTTPS filtering mostly. I think this feature is too complicated for such easy enabling:) So I suppose we should offer user to learn more about this feature instead of simply turning it on.

When doing such an ignoring/gray-out, it'd be helpful to put a note w/ the gray-out explaining which setting is causing the ignoring, e.g., "Global HTTPS-filtering is disabled" or "Wi-Fi is disabled for this app."

Yep, you're right, thank you! Assigning this for future release:
#699

Member

ameshkov commented Jul 18, 2016

attempting to enable HTTPS-filtering for an app where ad-filtering is disabled shouldn't fail — it should enable both HTTPS- & ad-filtering, to make clear that requirement

It will look strange if we make grayed out item usable.

WiFi ↔ Wifi when screen off
Cellular data ↔ Cellular data when screen off

Yep, this should be done:
#697

Firewall notifications when app traffic is routed through AG, but (inexplicably) no other notifiable options are enabled

Didn't get that point, what do you mean?

when HTTPS-filtering isn't enabled @ all, the per app settings shouldn't be disabled, but grayed out (potential to keep previous setting) to show it's being ignored

#698

Attempting to enable these per-app shouldn't immediately enable the global setting, but put ↑ a box asking

I guess this is about HTTPS filtering mostly. I think this feature is too complicated for such easy enabling:) So I suppose we should offer user to learn more about this feature instead of simply turning it on.

When doing such an ignoring/gray-out, it'd be helpful to put a note w/ the gray-out explaining which setting is causing the ignoring, e.g., "Global HTTPS-filtering is disabled" or "Wi-Fi is disabled for this app."

Yep, you're right, thank you! Assigning this for future release:
#699

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jul 18, 2016

Contributor

It will look strange if we make grayed out item usable.

@ameshkov You're probably correct, & maybe #699 would be enough. Still, it'd be a cool hack. 😉

& thanks for taking the rest so well! 😊

Contributor

TPS commented Jul 18, 2016

It will look strange if we make grayed out item usable.

@ameshkov You're probably correct, & maybe #699 would be enough. Still, it'd be a cool hack. 😉

& thanks for taking the rest so well! 😊

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jul 18, 2016

Contributor

Firewall notifications when app traffic is routed through AG, but (inexplicably) no other notifiable options are enabled

Didn't get that point, what do you mean?

@ameshkov I mean, if both wifi & mobile-data (& their -screen-equivalents) are left enabled for an app, there's nothing to firewall-notify, right? So the option should be grayed-out/ignored.

Contributor

TPS commented Jul 18, 2016

Firewall notifications when app traffic is routed through AG, but (inexplicably) no other notifiable options are enabled

Didn't get that point, what do you mean?

@ameshkov I mean, if both wifi & mobile-data (& their -screen-equivalents) are left enabled for an app, there's nothing to firewall-notify, right? So the option should be grayed-out/ignored.

@TPS

This comment has been minimized.

Show comment
Hide comment
@TPS

TPS Jul 18, 2016

Contributor

I guess this is about HTTPS filtering mostly.

@ameshkov Since I don't use the HTTPS filtering myself, I was thinking of mobile data (blocked globally by me), & I could see others wanting such for wifi, as well.

Contributor

TPS commented Jul 18, 2016

I guess this is about HTTPS filtering mostly.

@ameshkov Since I don't use the HTTPS filtering myself, I was thinking of mobile data (blocked globally by me), & I could see others wanting such for wifi, as well.

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 19, 2016

Member

@ameshkov I mean, if both wifi & mobile-data (& their -screen-equivalents) are left enabled for an app, there's nothing to firewall-notify, right? So the option should be grayed-out/ignored.

You are right. I just keep in mind that we could add more "firewall-related" notifications in future and this setting should control them all.

Member

ameshkov commented Jul 19, 2016

@ameshkov I mean, if both wifi & mobile-data (& their -screen-equivalents) are left enabled for an app, there's nothing to firewall-notify, right? So the option should be grayed-out/ignored.

You are right. I just keep in mind that we could add more "firewall-related" notifications in future and this setting should control them all.

@ameshkov

This comment has been minimized.

Show comment
Hide comment
@ameshkov

ameshkov Jul 19, 2016

Member

@ameshkov Since I don't use the HTTPS filtering myself, I was thinking of mobile data (blocked globally by me), & I could see others wanting such for wifi, as well.

Bringing up a dialog every time will be too annoying. However, it would be good to somehow mark that the value is overridden or got from defaults.

Member

ameshkov commented Jul 19, 2016

@ameshkov Since I don't use the HTTPS filtering myself, I was thinking of mobile data (blocked globally by me), & I could see others wanting such for wifi, as well.

Bringing up a dialog every time will be too annoying. However, it would be good to somehow mark that the value is overridden or got from defaults.

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