Skip to content
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

Optimization AppManager UI #2335

Merged
merged 3 commits into from Dec 9, 2019
Merged

Optimization AppManager UI #2335

merged 3 commits into from Dec 9, 2019

Conversation

@shadowsocksRb
Copy link
Contributor

shadowsocksRb commented Oct 28, 2019

(cherry picked from commit 8306daa)
提交修改至上游

  1. 显示包括停用(disable)和隐藏(hide)状态的应用。这些状态可能由冰箱等“冻结“省电类应用设置。
  2. 排序优先显示用户应用。不管是否为绕过,用户应用与系统应用的代理策略总是差别很大。
  3. 应用到所有配置文件时,同步设置分应用开关和绕过状态。找不到不这样做的理由。
  4. 去掉了内容奇怪的错误提示,并且几乎不可能发生。
(cherry picked from commit 8306daa)
@madeye madeye requested a review from Mygod Oct 28, 2019
Copy link
Contributor

Mygod left a comment

Looks good except for some local comments.

@@ -132,7 +135,7 @@ class AppManager : AppCompatActivity() {
apps = getCachedApps(packageManager).map { (packageName, packageInfo) ->
coroutineContext[Job]!!.ensureActive()
ProxiedApp(packageManager, packageInfo.applicationInfo, packageName)
}.sortedWith(compareBy({ !isProxiedApp(it) }, { it.name.toString() }))
}.sortedWith(compareBy({ !isProxiedApp(it) }, { isSystemApp(it) }, { it.name.toString() }))

This comment has been minimized.

Copy link
@Mygod

Mygod Oct 28, 2019

Contributor

I think putting disabled/hidden components at the end of the list would also make sense.

This comment has been minimized.

Copy link
@Mygod

Mygod Oct 28, 2019

Contributor

Also you should just use { it.appInfo.flags and ApplicationInfo.FLAG_SYSTEM } instead of { isSystemApp(it) }.

This comment has been minimized.

Copy link
@shadowsocksRb

shadowsocksRb Oct 29, 2019

Author Contributor

有一个想法,商量一下。不按名字排序,改成按安装时间排序。
按名字排序只是看着整齐,但对使用帮助不大,对某个应用有明确要求前来更改时,会使用搜索而不是滑列表。而按安装时间排序可以快速调整近期新添加的应用,这也可以解决 #1635

This comment has been minimized.

Copy link
@shadowsocksRb

shadowsocksRb Oct 29, 2019

Author Contributor

判断是否为隐藏状态有点麻烦,这需要拿到privateFlags,而它有hide注解。

This comment has been minimized.

Copy link
@shadowsocksRb

shadowsocksRb Oct 30, 2019

Author Contributor

认真考虑后,将停用和隐藏应用放置在末尾不是个好主意。一般这类应用都是国产,在绕过模式下是需要被选择的应用,反而应该放在前面。
出厂就停用的应用需要MATCH_DISABLED_UNTIL_USED_COMPONENTS标志才能包含,它们已经被排除。
不特殊处理停用和隐藏应用。

This comment has been minimized.

Copy link
@Mygod

Mygod Dec 9, 2019

Contributor

It might be a good idea to have multiple sort options. For now, I think the old option makes the most sense.

This comment has been minimized.

Copy link
@shadowsocksRb

shadowsocksRb Dec 10, 2019

Author Contributor

虽然在提交日志中是Show disabled apps, 但去掉了包括disabled apps的flag, 仅保留了包括隐藏应用的flag……

@Mygod
Mygod approved these changes Dec 9, 2019
@Mygod Mygod merged commit 5da2fef into shadowsocks:master Dec 9, 2019
1 of 2 checks passed
1 of 2 checks passed
ci/circleci CircleCI is running your tests
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
@Mygod

This comment has been minimized.

Copy link
Contributor

Mygod commented Dec 9, 2019

Merged this. Sorry for the delay and thank you for the contribution!

As mentioned in #2335 (comment) you are welcome to improve this further by introducing multiple sort options, perhaps in a separate pull request.

@shadowsocksRb shadowsocksRb deleted the shadowsocksRb:ss-patch-2 branch Dec 10, 2019
@shadowsocksRb

This comment has been minimized.

Copy link
Contributor Author

shadowsocksRb commented Dec 10, 2019

在这个地方给用户太多选择不是个好主意,绝大多数人都只会使用默认排序。
Rb仅保留按安装时间排序和用户应用优先。

会再开一个PR添加包括停用应用和用户应用优先,这两个没有什么异议。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.