Chromium Code Reviews
Help | Chromium Project | Sign in
(56)

Issue 11886104: History: Add range navigation control for grouped visits (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 5 months ago by Sergiu
Modified:
2 years, 5 months ago
CC:
chromium-reviews, arv
Visibility:
Public.

Description

History: Add range navigation control for grouped visits The current patch enables the user to select the time frame for which the the grouped results are viewed by adding a set of buttons. These buttons allow the user to go back in time, reset to today or go forward until the current interval. BUG=170690 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180715

Patch Set 1 #

Patch Set 2 : Rebase #

Total comments: 4

Patch Set 3 : Rebase #

Patch Set 4 : Getters setters and minor edits. #

Patch Set 5 : Rebase #

Patch Set 6 : Rebase #

Patch Set 7 : Fix minor bug #

Total comments: 6

Patch Set 8 : Rebase with the other commit and fix some things. #

Total comments: 3

Patch Set 9 : Replies to comments. #

Total comments: 2

Patch Set 10 : Rebase #

Patch Set 11 : Fix the tests and make the last fixes #

Patch Set 12 : Fix so that you can only have an offset request at a time. #

Total comments: 6

Patch Set 13 : Last edits #

Messages

Total messages: 21 (0 generated)
Sergiu
Aaaand we're done, please take a look at this as well. Thanks, Sergiu
2 years, 5 months ago (2013-01-17 18:56:38 UTC) #1
Bernhard Bauer
https://codereview.chromium.org/11886104/diff/3001/chrome/browser/resources/history/history.html File chrome/browser/resources/history/history.html (right): https://codereview.chromium.org/11886104/diff/3001/chrome/browser/resources/history/history.html#newcode53 chrome/browser/resources/history/history.html:53: <button id="range-previous" disabled> This should fit on one line. ...
2 years, 5 months ago (2013-01-18 16:08:27 UTC) #2
Sergiu
https://codereview.chromium.org/11886104/diff/3001/chrome/browser/resources/history/history.html File chrome/browser/resources/history/history.html (right): https://codereview.chromium.org/11886104/diff/3001/chrome/browser/resources/history/history.html#newcode53 chrome/browser/resources/history/history.html:53: <button id="range-previous" disabled> On 2013/01/18 16:08:27, Bernhard Bauer wrote: ...
2 years, 5 months ago (2013-01-21 14:05:24 UTC) #3
Bernhard Bauer
lgtm
2 years, 5 months ago (2013-01-21 15:24:45 UTC) #4
Sergiu
Hey, this is the last patch of this series, please take a look. Thanks.
2 years, 5 months ago (2013-01-22 12:21:49 UTC) #5
Sergiu
On 2013/01/22 12:21:49, Sergiu wrote: > Hey, this is the last patch of this series, ...
2 years, 5 months ago (2013-01-28 09:59:44 UTC) #6
James Hawkins
https://codereview.chromium.org/11886104/diff/21004/chrome/browser/resources/history/history.html File chrome/browser/resources/history/history.html (right): https://codereview.chromium.org/11886104/diff/21004/chrome/browser/resources/history/history.html#newcode53 chrome/browser/resources/history/history.html:53: <button id="range-previous" disabled>◀</button> I'm not sure we should be ...
2 years, 5 months ago (2013-01-28 17:48:15 UTC) #7
Sergiu
I've addressed some of James' comments. I'm still not sure about the < and > ...
2 years, 5 months ago (2013-01-30 16:09:39 UTC) #8
James Hawkins
LGTM with question. https://codereview.chromium.org/11886104/diff/36001/chrome/browser/resources/history/history.js File chrome/browser/resources/history/history.js (right): https://codereview.chromium.org/11886104/diff/36001/chrome/browser/resources/history/history.js#newcode491 chrome/browser/resources/history/history.js:491: Object.defineProperty(HistoryModel.prototype, 'offset', { What is the ...
2 years, 5 months ago (2013-01-30 16:30:09 UTC) #9
Sergiu
https://codereview.chromium.org/11886104/diff/36001/chrome/browser/resources/history/history.js File chrome/browser/resources/history/history.js (right): https://codereview.chromium.org/11886104/diff/36001/chrome/browser/resources/history/history.js#newcode491 chrome/browser/resources/history/history.js:491: Object.defineProperty(HistoryModel.prototype, 'offset', { On 2013/01/30 16:30:09, James Hawkins wrote: ...
2 years, 5 months ago (2013-01-30 17:07:13 UTC) #10
Patrick Dubroy
https://codereview.chromium.org/11886104/diff/31001/chrome/browser/resources/history/history.js File chrome/browser/resources/history/history.js (right): https://codereview.chromium.org/11886104/diff/31001/chrome/browser/resources/history/history.js#newcode768 chrome/browser/resources/history/history.js:768: window.scrollTo(0, 0); I'm having deja vu...I swear I wrote ...
2 years, 5 months ago (2013-01-30 23:25:33 UTC) #11
Sergiu
On 2013/01/30 23:25:33, dubroy wrote: > https://codereview.chromium.org/11886104/diff/31001/chrome/browser/resources/history/history.js > File chrome/browser/resources/history/history.js (right): > > https://codereview.chromium.org/11886104/diff/31001/chrome/browser/resources/history/history.js#newcode768 > ...
2 years, 5 months ago (2013-01-30 23:34:51 UTC) #12
Patrick Dubroy
On 2013/01/30 23:34:51, Sergiu wrote: > On 2013/01/30 23:25:33, dubroy wrote: > > > https://codereview.chromium.org/11886104/diff/31001/chrome/browser/resources/history/history.js ...
2 years, 5 months ago (2013-01-30 23:42:08 UTC) #13
Sergiu
On 2013/01/30 23:42:08, dubroy wrote: > On 2013/01/30 23:34:51, Sergiu wrote: > > On 2013/01/30 ...
2 years, 5 months ago (2013-02-04 19:58:16 UTC) #14
Patrick Dubroy
lgtm with nits. On 2013/02/04 19:58:16, Sergiu wrote: > On 2013/01/30 23:42:08, dubroy wrote: > ...
2 years, 5 months ago (2013-02-04 23:52:02 UTC) #15
Patrick Dubroy
lgtm with nits. https://codereview.chromium.org/11886104/diff/51001/chrome/browser/resources/history/history.js File chrome/browser/resources/history/history.js (right): https://codereview.chromium.org/11886104/diff/51001/chrome/browser/resources/history/history.js#newcode523 chrome/browser/resources/history/history.js:523: // Skip |offset_| * weeks/months from ...
2 years, 5 months ago (2013-02-04 23:52:20 UTC) #16
Sergiu
Thanks! Flipping the commit bit. https://codereview.chromium.org/11886104/diff/51001/chrome/browser/resources/history/history.js File chrome/browser/resources/history/history.js (right): https://codereview.chromium.org/11886104/diff/51001/chrome/browser/resources/history/history.js#newcode523 chrome/browser/resources/history/history.js:523: // Skip |offset_| * ...
2 years, 5 months ago (2013-02-05 09:45:18 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergiu@chromium.org/11886104/64001
2 years, 5 months ago (2013-02-05 09:46:42 UTC) #18
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) sync_integration_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=108218
2 years, 5 months ago (2013-02-05 14:25:57 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergiu@chromium.org/11886104/64001
2 years, 5 months ago (2013-02-05 15:07:22 UTC) #20
commit-bot: I haz the power
2 years, 5 months ago (2013-02-05 17:03:08 UTC) #21
Message was sent while issue was closed.
Change committed as 180715
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1f9106d