[MD History] Make forward/backward work in grouped mode.

[MD History] Make forward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/5530edba1faf254b998c041300f801739c6a6d5a
Cr-Commit-Position: refs/heads/master@{#435547}

[calamity, 2016-11-07 02:46:33]

Description was changed from

==========
[MD History] Make foward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
==========

to

==========
[MD History] Make foward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[calamity, 2016-11-10 01:52:27]

Patchset #1 (id:1) has been deleted

[calamity, 2016-11-10 01:52:35]

Patchset #1 (id:20001) has been deleted

[calamity, 2016-11-10 03:18:55]

Patchset #1 (id:40001) has been deleted

[calamity, 2016-11-10 03:19:31]

calamity@chromium.org changed reviewers:
+ tsergeant@chromium.org

[tsergeant, 2016-11-11 02:34:27]

Yay, I'm glad that databinding made this relatively easy to do.

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_toolbar.html (right):

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.html:234: <span
id="grouped-date" hidden$="[[querying]]">
IMO, this hiding is unnecessary.

It just causes the date to flash in and out distractingly. Without it, the dates
for the old results should still display until the new results are ready. This
mirrors the behavior of the main view, where the old results show until the new
ones appear.

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.html:237: <paper-icon-button
id="today-button" icon="history:today"
Do these buttons actually need IDs?

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.html:240:
on-tap="todayTapped_"
Nit: Follow naming style of onTodayTap_ here and below

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_toolbar.js (right):

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.js:57: queryFinished:
Boolean,
I was a bit confused by this name, because it sounds like it's about an
individual query rather than whether the user is at the end of available
results.

It would be good to add a comment to explain, or rename to something clearer.
Strawman alternative suggestion: `resultsFinished`?

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.js:142: hasDrawerChanged_:
function() { this.updateStyles(); },
While you're here...

I'm pretty sure this listener can be deleted now that we've moved to native
custom properties.

(It was necessary to ensure that the search field is positioned correctly when
the sidebar shows/hides)

[calamity, 2016-11-22 06:44:31]

The CQ bit was checked by calamity@chromium.org to run a CQ dry run

[calamity, 2016-11-22 06:44:40]

Patchset #3 (id:100001) has been deleted

[commit-bot: I haz the power, 2016-11-22 06:44:46]

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

[calamity, 2016-11-22 06:44:47]

Patchset #3 (id:120001) has been deleted

[calamity, 2016-11-22 06:44:56]

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_toolbar.html (right):

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.html:234: <span
id="grouped-date" hidden$="[[querying]]">
On 2016/11/11 02:34:27, tsergeant wrote:
> IMO, this hiding is unnecessary.
> 
> It just causes the date to flash in and out distractingly. Without it, the
dates
> for the old results should still display until the new results are ready. This
> mirrors the behavior of the main view, where the old results show until the
new
> ones appear.

Done.

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.html:237: <paper-icon-button
id="today-button" icon="history:today"
On 2016/11/11 02:34:27, tsergeant wrote:
> Do these buttons actually need IDs?

Just for tests?

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.html:240:
on-tap="todayTapped_"
On 2016/11/11 02:34:27, tsergeant wrote:
> Nit: Follow naming style of onTodayTap_ here and below

Done.

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_toolbar.js (right):

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.js:57: queryFinished:
Boolean,
On 2016/11/11 02:34:27, tsergeant wrote:
> I was a bit confused by this name, because it sounds like it's about an
> individual query rather than whether the user is at the end of available
> results.
> 
> It would be good to add a comment to explain, or rename to something clearer.
> Strawman alternative suggestion: `resultsFinished`?
> 

hasMoreResults is probably optimally clear. Even with the double negating....

https://codereview.chromium.org/2481693002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.js:142: hasDrawerChanged_:
function() { this.updateStyles(); },
On 2016/11/11 02:34:27, tsergeant wrote:
> While you're here...
> 
> I'm pretty sure this listener can be deleted now that we've moved to native
> custom properties.
> 
> (It was necessary to ensure that the search field is positioned correctly when
> the sidebar shows/hides)

Done.

[commit-bot: I haz the power, 2016-11-22 07:26:46]

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

[commit-bot: I haz the power, 2016-11-22 07:26:46]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[tsergeant, 2016-11-23 05:46:48]

Generally looks good, but the tests are currently failing

https://codereview.chromium.org/2481693002/diff/140001/chrome/test/data/webui...
File chrome/test/data/webui/md_history/history_toolbar_test.js (left):

https://codereview.chromium.org/2481693002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/md_history/history_toolbar_test.js:98:
app.set('queryState_.searchTerm', '');
This line is necessary to remove the ?q= from the URL. Otherwise, the query will
show up again when replaceApp() is called. I assume this is what is causing the
tests to fail.

(We probably should just reset the query state in the URL as part of replaceApp
to ensure this can never happen)

[calamity, 2016-11-24 06:45:10]

The CQ bit was checked by calamity@chromium.org to run a CQ dry run

[calamity, 2016-11-24 06:45:18]

https://codereview.chromium.org/2481693002/diff/140001/chrome/test/data/webui...
File chrome/test/data/webui/md_history/history_toolbar_test.js (left):

https://codereview.chromium.org/2481693002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/md_history/history_toolbar_test.js:98:
app.set('queryState_.searchTerm', '');
On 2016/11/23 05:46:48, tsergeant wrote:
> This line is necessary to remove the ?q= from the URL. Otherwise, the query
will
> show up again when replaceApp() is called. I assume this is what is causing
the
> tests to fail.
> 
> (We probably should just reset the query state in the URL as part of
replaceApp
> to ensure this can never happen)

Good catch. Done.

[commit-bot: I haz the power, 2016-11-24 06:45:40]

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

[tsergeant, 2016-11-24 06:52:52]

lgtm

[commit-bot: I haz the power, 2016-11-24 08:47:03]

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

[commit-bot: I haz the power, 2016-11-24 08:47:04]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  closure_compilation on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[calamity, 2016-11-30 06:55:26]

Description was changed from

==========
[MD History] Make foward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD History] Make forward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[calamity, 2016-12-01 02:37:26]

The CQ bit was checked by calamity@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-01 02:37:42]

Dry run: 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-12-01 03:36:17]

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

[commit-bot: I haz the power, 2016-12-01 03:36:18]

Dry run: This issue passed the CQ dry run.

[calamity, 2016-12-01 03:37:44]

The CQ bit was checked by calamity@chromium.org

[calamity, 2016-12-01 03:37:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
tsergeant@chromium.org
Link to the patchset: https://codereview.chromium.org/2481693002/#ps180001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-01 03:37:57]

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-12-01 03:45:50]

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

[commit-bot: I haz the power, 2016-12-01 03:45:51]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[calamity, 2016-12-01 03:51:06]

calamity@chromium.org changed reviewers:
+ dbeam@chromium.org

[calamity, 2016-12-01 03:51:06]

TBR-ing dbeam for md_downloads/vulcanized.html

[calamity, 2016-12-01 03:51:24]

Description was changed from

==========
[MD History] Make forward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD History] Make forward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[calamity, 2016-12-01 03:51:33]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2016-12-01 03:51:42]

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-12-01 03:54:27]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1480564293542290,
"parent_rev": "1e7377b3238e0c1200ed9748e0f6c82990788610", "commit_rev":
"edcc6d40e5c140c377f4bd24919665a96380aae9"}

[commit-bot: I haz the power, 2016-12-01 03:54:55]

Description was changed from

==========
[MD History] Make forward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD History] Make forward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-12-01 03:54:57]

Committed patchset #5 (id:180001)

[commit-bot: I haz the power, 2016-12-01 03:57:35]

Description was changed from

==========
[MD History] Make forward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD History] Make forward/backward work in grouped mode.

This CL hooks up the toolbar forward and backward buttons in grouped mode to
change the search timeframe for week and month queries.

BUG=589357
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/5530edba1faf254b998c041300f801739c6a6d5a
Cr-Commit-Position: refs/heads/master@{#435547}
==========

[commit-bot: I haz the power, 2016-12-01 03:57:36]

Patchset 5 (id:??) landed as
https://crrev.com/5530edba1faf254b998c041300f801739c6a6d5a
Cr-Commit-Position: refs/heads/master@{#435547}

[Dan Beam, 2016-12-01 03:58:09]

er, md_downloads lgtm i suppose