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

分应用界面优化 #2139

Merged
merged 26 commits into from Mar 8, 2019

Conversation

Projects
None yet
4 participants
@CzBiX
Copy link
Contributor

commented Mar 1, 2019

close #2138

  1. 显式显示 Android System
  2. 显示 uid 和 package name
  3. 加入快速搜索功能
  4. 同步相同 uid 的选择状态

界面效果如图:
image

@Mygod
Copy link
Contributor

left a comment

Thanks. I'll look at it some more later.

Show resolved Hide resolved core/src/main/res/values-zh-rCN/strings.xml Outdated
Show resolved Hide resolved mobile/src/main/res/layout/layout_apps_item.xml
@Mygod
Copy link
Contributor

left a comment

The color for radio buttons seems off. Can you correct that?

Other than that, it would be cool to move the search into toolbar.

@madeye

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

Any update on this? @CzBiX

@CzBiX

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

Sorry for the delay, I'll update at night.

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

No hurries. Take your time.

@CzBiX CzBiX force-pushed the CzBiX:ss_cn branch from 779b3b9 to 39a1550 Mar 7, 2019

@madeye madeye requested a review from Mygod Mar 8, 2019

@madeye

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

I think all the issues are resolved now? @Mygod

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

I will pull the latest commit down soon and take another look at the changes.

@Mygod Mygod added the enhancement label Mar 8, 2019

@Mygod
Copy link
Contributor

left a comment

On my device, uid 1000 is shared with "Settings", which explicitly requires INTERNET (therefore it will be granted to Android system as well). It seems like we do not need to explicitly include Android system and it's sufficient to have Settings only. Could you check if that works?

Also, I believe we can safely assume these for Android settings as they have been there since the very beginning. We probably do not need to worry about TV unless somebody complains about some samsung TV.

@Mygod Mygod self-requested a review Mar 8, 2019

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Also I don't think importing works.

Mygod added some commits Mar 8, 2019

@Mygod

Mygod approved these changes Mar 8, 2019

@Mygod Mygod requested a review from madeye Mar 8, 2019

@CzBiX

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

我觉得把为 Settings 启用代理从而导致系统也走代理这样的副作用当作解决问题的方式只能算是 workaround。

@ayanamist

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

同意,而且基本没有人会想得到这点,也算的上hidden gotcha了

@madeye

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Maybe we can add a custom entry, which called "System" with a uid 1000.

@madeye

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Okay, I see the problem here.

Let's revert 02b915f. I think it's better to show android explicitly.

madeye added some commits Mar 8, 2019

@madeye

madeye approved these changes Mar 8, 2019

@madeye madeye merged commit e3a5e00 into shadowsocks:master Mar 8, 2019

1 of 2 checks passed

Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
ci/circleci Your tests passed on CircleCI!
Details
@madeye

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Merged. Thanks!

monitor.wait(channel, SelectionKey.OP_READ)
if (channel.receive(result) == proxy) break
result.clear()
}

This comment has been minimized.

Copy link
@Mygod

Mygod Mar 8, 2019

Contributor

Hmmm is this a rebase?

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