Skip to content
This repository has been archived by the owner on Sep 6, 2019. It is now read-only.

Flattened hierarchy of mainentry #1933

Closed
wants to merge 4 commits into from
Closed

Conversation

tonymanou
Copy link
Contributor

This improves performances and allow us to get rid of the NestedWeights warning.
The main list now scrolls slightly faster.

@tonymanou
Copy link
Contributor Author

I also tuned a padding and a margin in the main list.

@M66B M66B closed this in 2abd21d Aug 28, 2014
@M66B
Copy link
Owner

M66B commented Aug 28, 2014

Good find!
This is possible since the recent UI changes.
Formerly the linear layout handled a tap / was used for showing a highlight.

@tonymanou
Copy link
Contributor Author

You did not include commits a3b3007 and c73e4f4, aren't you interested by the padding/margin fix?

@M66B
Copy link
Owner

M66B commented Aug 28, 2014

I am not sure what the fix is needed for and I didn't wanted to take risks for the upcoming stable version.

XPrivacy uses a grid of a multiple of 3 or 6 dip.

@tonymanou
Copy link
Contributor Author

The margin fix only affects small and medium screens (you can see on the right of the list).
The padding fix affects all screen sizes (you can see that the gap on the left of app's icon in the list).

Medium screens (padding and margin fixes)

ab_small

Large screens (padding fix)

ab_large

It's up to you! The padding fix is not mandatory, but the margin fix is necessary for non-large screens.

@M66B
Copy link
Owner

M66B commented Aug 28, 2014

I will look later into the padding and margin fixes.

@tonymanou
Copy link
Contributor Author

I forgot to fix the right margin of details view's list for small and medium screens.
Here it is!

@M66B
Copy link
Owner

M66B commented Aug 30, 2014

Can you please post screenshots of before and after the fix?

@tonymanou
Copy link
Contributor Author

Sure!

Medium screen

small_screen

Large screen

big_screen

@@ -5,6 +5,6 @@
screen margins) for sw720dp devices (e.g. 10" tablets) in landscape here.
-->
<dimen name="activity_horizontal_margin">128dp</dimen>
<dimen name="activity_horizontal_margin_list">106dp</dimen>
<dimen name="activity_horizontal_margin_list">112dp</dimen>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

106 dip is Android default, why needs this to be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reduction of the list's right padding affects all screen sizes, but graphical render for large screens was already good before the fix. Thus we need to counter-balance the right padding reduction for large screens.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to keep the Android default.
This can never be a big deal om large screens.
The other margin fixes are applied now (unless I missed something) and it looks indeed nicer and makes a little bit more room for the application / restriction names.
Thanks for bringing this to my attention!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well...

large_bug

activity_horizontal_margin_list was already a custom implementation (compared with activity_horizontal_margin). Why not tune it up a bit if it fixes a graphical glitch?

M66B added a commit that referenced this pull request Aug 30, 2014
@M66B M66B closed this Aug 30, 2014
M66B added a commit that referenced this pull request Aug 30, 2014
@M66B
Copy link
Owner

M66B commented Aug 30, 2014

I cannot test this, so I trust it is okay.

@tonymanou
Copy link
Contributor Author

Thanks!
FYI I used a 10" emulator (Genymotion particularly) to test XPrivacy's rendering for large screens.

@tonymanou tonymanou deleted the mainlist branch August 30, 2014 13:18
@M66B
Copy link
Owner

M66B commented Aug 30, 2014

I should thank you ...
I have accepted your invitation on GTalk ...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants