[Android] Introduce DateDividedAdapter to Chrome

[Android] Introduce DateDividedAdapter to Chrome

This CL brings DateDividedAdapter to Chrome, and lets Download Manager
UI to utilize it. This adapter will take a list of timed items, groups
these items by date, and displays the groups in sections.

BUG=616324
Committed: https://crrev.com/ba44cdfb271df1296a78b81f91959bbeb4f4b309
Cr-Commit-Position: refs/heads/master@{#406641}

[Ian Wen, 2016-07-16 05:02:53]

ianwen@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Ian Wen, 2016-07-16 05:02:53]

PTAL :)

[gone, 2016-07-18 19:09:49]

Can you upload some screenshots?

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
(right):

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:79:
holder.mHostnameView.setText(UrlUtilities
Guess you prefer to move this up?  I kind of liked it below because it kept the
entire function call on one line.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java
(right):

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:65:

extra newline

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:68:
mTextView.setText(mTextView.getContext().getString(R.string.today));
The format is Today - Day of week - DATE

Maybe

StringBuilder labelBuilder = new StringBuilder();

if (compareCalendar(cal1, cal2) == 0) {
    // Check if the date is today's.
    labelBuilder.append(mTextView.getContext().getString(R.string.today));
    labelBuilder.append(" - ");
} else {
    // Check if the date is yesterday's.
    cal1.add(Calendar.DATE, -1);
    if (compareCalendar(cal1, cal2) == 0){
       
labelBuilder.append(mTextView.getContext().getString(R.string.yesterday));
        labelBuilder.append(" - ");
    }
}

labelBuilder.append(
        DateUtils.formatDateTime(mTextView.getContext(), date.getTime(),
                DateUtils.FORMAT_SHOW_WEEKDAY | DateUtils.FORMAT_SHOW_DATE
                | DateUtils.FORMAT_SHOW_YEAR));
mTextView.setText(labelBuilder.toString());

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:84:
* A sublist of items, grouped by date.
A bucket of items with the same date?

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:103:
public boolean sameDay(Date otherDate) {
isSameDay

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:122:
// Cached async tasks to get the two Calendar objets, which are used when
comparing dates.
objets -> objects, private static final

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:159:
clear();
We probably don't want to trigger a notifyDataSetChanged() immediately after the
clear since we're doing another one at the end of this function.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:171:
if (!found) {
// Create a new ItemGroup with the date for the new item.  This increases the
// size by two because we add new views for the date and the item itself.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:194:
for (ItemGroup group : mItems) {
Kind of unfortunate that his for loop will get worse as time goes on.  Might
want to add a TODO() to keep a summed list of bucket sizes or something.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:212:
for (ItemGroup group : mItems) {
This has similar logic to the above; maybe it should be pulled out into a
function that returns a Pair<ItemGroup, Integer>, where the ItemGroup is the one
that is identified and Integer is the value of i?

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:245:
bindViewHolder(holder, pair.second);
bindViewHolderForTimedItem?

[gone, 2016-07-18 19:10:47]

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java
(right):

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:68:
mTextView.setText(mTextView.getContext().getString(R.string.today));
On 2016/07/18 19:09:48, dfalcantara wrote:
> The format is Today - Day of week - DATE
> 
> Maybe
> 
> StringBuilder labelBuilder = new StringBuilder();
> 
> if (compareCalendar(cal1, cal2) == 0) {
>     // Check if the date is today's.
>     labelBuilder.append(mTextView.getContext().getString(R.string.today));
>     labelBuilder.append(" - ");
> } else {
>     // Check if the date is yesterday's.
>     cal1.add(Calendar.DATE, -1);
>     if (compareCalendar(cal1, cal2) == 0){
>        
> labelBuilder.append(mTextView.getContext().getString(R.string.yesterday));
>         labelBuilder.append(" - ");
>     }
> }
> 
> labelBuilder.append(
>         DateUtils.formatDateTime(mTextView.getContext(), date.getTime(),
>                 DateUtils.FORMAT_SHOW_WEEKDAY | DateUtils.FORMAT_SHOW_DATE
>                 | DateUtils.FORMAT_SHOW_YEAR));
> mTextView.setText(labelBuilder.toString());

Er, the second dash is supposed to be a comma apparently.  Check about:history
on desktop.

[Ian Wen, 2016-07-19 22:29:38]

PTAL

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
(right):

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:79:
holder.mHostnameView.setText(UrlUtilities
On 2016/07/18 19:09:48, dfalcantara wrote:
> Guess you prefer to move this up?  I kind of liked it below because it kept
the
> entire function call on one line.

Nah this was done by eclipse. I've no preference :P

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java
(right):

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:65:

On 2016/07/18 19:09:48, dfalcantara wrote:
> extra newline

Done.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:84:
* A sublist of items, grouped by date.
On 2016/07/18 19:09:48, dfalcantara wrote:
> A bucket of items with the same date?

Aha that's why I need to take TOEFL test before entering an American school.
Done.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:103:
public boolean sameDay(Date otherDate) {
On 2016/07/18 19:09:48, dfalcantara wrote:
> isSameDay

Done.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:122:
// Cached async tasks to get the two Calendar objets, which are used when
comparing dates.
On 2016/07/18 19:09:48, dfalcantara wrote:
> objets -> objects, private static final

Done.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:159:
clear();
On 2016/07/18 19:09:48, dfalcantara wrote:
> We probably don't want to trigger a notifyDataSetChanged() immediately after
the
> clear since we're doing another one at the end of this function.

Done.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:171:
if (!found) {
On 2016/07/18 19:09:48, dfalcantara wrote:
> // Create a new ItemGroup with the date for the new item.  This increases the
> // size by two because we add new views for the date and the item itself.

Done.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:194:
for (ItemGroup group : mItems) {
On 2016/07/18 19:09:48, dfalcantara wrote:
> Kind of unfortunate that his for loop will get worse as time goes on.  Might
> want to add a TODO() to keep a summed list of bucket sizes or something.

Done.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:212:
for (ItemGroup group : mItems) {
On 2016/07/18 19:09:48, dfalcantara wrote:
> This has similar logic to the above; maybe it should be pulled out into a
> function that returns a Pair<ItemGroup, Integer>, where the ItemGroup is the
one
> that is identified and Integer is the value of i?

Great suggestion. Done.

https://chromiumcodereview.appspot.com/2154063003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:245:
bindViewHolder(holder, pair.second);
On 2016/07/18 19:09:48, dfalcantara wrote:
> bindViewHolderForTimedItem?

Done.

[gone, 2016-07-19 22:40:00]

lgtm % the comment about clearing and updating the font spacings and styling.

https://chromiumcodereview.appspot.com/2154063003/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java
(right):

https://chromiumcodereview.appspot.com/2154063003/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:81:
android.util.Log.w("jokey:", "setDate(): " + builder.toString());
Leftover log

https://chromiumcodereview.appspot.com/2154063003/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/widget/DateDividedAdapter.java:161:
public void loadItems(List<? extends TimedItem> timedItems) {
Er, we still want to clear(), but I think that clear() calls
notifyDataSetChanged().  You might want to pass in a boolean into clear() or
something that prevents the notifyDataSetChanged() from being called.

[Ian Wen, 2016-07-19 23:21:15]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-07-19 23:21:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2154063003/#ps40001
(title: "bold->roboto-medium")

[commit-bot: I haz the power, 2016-07-19 23:21:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 00:10:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-20 00:10:04]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Ian Wen, 2016-07-20 18:31:45]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-07-20 18:31:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2154063003/#ps60001
(title: "make findbugs happy")

[commit-bot: I haz the power, 2016-07-20 18:32:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 19:42:32]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-20 19:42:53]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-20 19:44:51]

Description was changed from

==========
[Android] Introduce DateDividedAdapter to Chrome

This CL brings DateDividedAdapter to Chrome, and lets Download Manager
UI to utilize it. This adapter will take a list of timed items, groups
these items by date, and displays the groups in sections.

BUG=616324
==========

to

==========
[Android] Introduce DateDividedAdapter to Chrome

This CL brings DateDividedAdapter to Chrome, and lets Download Manager
UI to utilize it. This adapter will take a list of timed items, groups
these items by date, and displays the groups in sections.

BUG=616324

Committed: https://crrev.com/ba44cdfb271df1296a78b81f91959bbeb4f4b309
Cr-Commit-Position: refs/heads/master@{#406641}
==========

[commit-bot: I haz the power, 2016-07-20 19:44:52]

Patchset 4 (id:??) landed as
https://crrev.com/ba44cdfb271df1296a78b81f91959bbeb4f4b309
Cr-Commit-Position: refs/heads/master@{#406641}