Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(96)

Issue 2370203002: Demonstrate some ideas for level list drawable [DO NOT SUBMIT]

Created:
4 years, 2 months ago by Ian Wen
Modified:
4 years, 2 months ago
Reviewers:
ortuno
CC:
agrieve+watch_chromium.org, chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Demonstrate some ideas for level list drawable BUG=

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -10 lines) Patch
M chrome/android/java/res/layout/item_chooser_dialog_row.xml View 1 chunk +1 line, -0 lines 1 comment Download
M chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java View 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java View 4 chunks +24 lines, -2 lines 2 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java View 8 chunks +17 lines, -5 lines 1 comment Download
M chrome/android/java/src/org/chromium/chrome/browser/UsbChooserDialog.java View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (1 generated)
Ian Wen
4 years, 2 months ago (2016-09-27 04:23:09 UTC) #2
ortuno
4 years, 2 months ago (2016-09-27 07:02:30 UTC) #3
Thanks for this! I really appreciate it.

https://codereview.chromium.org/2370203002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/item_chooser_dialog_row.xml (right):

https://codereview.chromium.org/2370203002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/item_chooser_dialog_row.xml:12:
android:drawablePadding="3dp"
Regarding compound drawables. I'm not really opposed to the idea but in the end
the row will consist of two lines and an icon. You can see the mock here[1]
under "Paired but not connected".

[1] https://drive.google.com/file/d/0B-TxIvn0COW1eWd2VWtRSmM0YjA/view

Would compound drawable still make sense?

https://codereview.chromium.org/2370203002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
(right):

https://codereview.chromium.org/2370203002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java:1596: //
This should be very cheap, because actual drawables are shared by the same
constant state.
Just to confirm. We are updating the rows approximately 3000 times in a 10
second period. That's 3000 new Drawables that we will be creating. Is it still
OK to do this?

https://codereview.chromium.org/2370203002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java:1598:
d.setLevel(1);
I'm not sure if I got the code wrong but for some reason when I do the following
the icon shows the level 0 image:

Drawable d = lld.getConstantState().newDrawable();
d.setLevel(1);
icon.setImageDrawable(d);

Whereas if I do the following, the icon displays the right image:

Drawable d = lld.getConstantState().newDrawable();
icon.setImageDrawable(d);
d.setLevel(1);

What am I missing?

https://codereview.chromium.org/2370203002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
(right):

https://codereview.chromium.org/2370203002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java:74:
mDrawable = drawable;
The main reason for the ItemChooserIcon object was the fact that we need a pair
of icons: one for when the row is not selected (grey) and one for when the row
is selected (white). Do you suggest I just keep them separate?

Powered by Google App Engine
This is Rietveld 408576698