Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2618633002:
[MD History] Make first URL on Synced Device page instead of menu button. (Closed)
|
Created:
3 years, 11 months ago by calamity Modified:
3 years, 11 months ago CC:
chromium-reviews, Patrick Dubroy, michaelpg+watch-md-ui_chromium.org, dbeam+watch-history_chromium.org, pam+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, chrome-apps-syd-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[MD History] Make first URL on Synced Device page instead of menu button.
BUG=653398
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2618633002
Cr-Commit-Position: refs/heads/master@{#441884}
Committed: https://chromium.googlesource.com/chromium/src/+/a4ecd5b7dee7b4ea7444dee445f9757a34d43564
Patch Set 1 #
Total comments: 6
Patch Set 2 : address_comments; #Patch Set 3 : fix_closure #
Total comments: 1
Messages
Total messages: 20 (12 generated)
Description was changed from ========== [MD History] Make first URL on Synced Device page instead of menu button. BUG= ========== to ========== [MD History] Make first URL on Synced Device page instead of menu button. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== [MD History] Make first URL on Synced Device page instead of menu button. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [MD History] Make first URL on Synced Device page instead of menu button. BUG=653398 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
calamity@chromium.org changed reviewers: + tsergeant@chromium.org
Thoughts?
Cool! Good thing to fix. https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... File ui/webui/resources/js/cr/ui/focus_grid.js (right): https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... ui/webui/resources/js/cr/ui/focus_grid.js:152: ensureRowActive: function(preferredRow) { Optional parameters should start with opt_ (see: https://google.github.io/styleguide/javascriptguide.xml?showone=Naming#Naming)
sorry for the drive-by https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... File ui/webui/resources/js/cr/ui/focus_grid.js (right): https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... ui/webui/resources/js/cr/ui/focus_grid.js:150: * active. Selects the first item if this is beyond the range of the grid. nit: indent a little more https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... ui/webui/resources/js/cr/ui/focus_grid.js:152: ensureRowActive: function(preferredRow) { On 2017/01/06 00:33:50, tsergeant wrote: > Optional parameters should start with opt_ > > (see: > https://google.github.io/styleguide/javascriptguide.xml?showone=Naming#Naming) welllll, the ES6 version says not to do this. so i assume it's going away fairly soon https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... ui/webui/resources/js/cr/ui/focus_grid.js:164: this.rows[preferredRow].makeActive(true); (this.rows[preferredRow] || this.rows[0]).makeActive(true);
calamity@chromium.org changed reviewers: + dbeam@chromium.org
+dbeam for focus_grid.js OWNERS https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... File ui/webui/resources/js/cr/ui/focus_grid.js (right): https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... ui/webui/resources/js/cr/ui/focus_grid.js:150: * active. Selects the first item if this is beyond the range of the grid. On 2017/01/06 00:45:29, Dan Beam wrote: > nit: indent a little more Done. Some men just want to watch the world wrap. https://codereview.chromium.org/2618633002/diff/1/ui/webui/resources/js/cr/ui... ui/webui/resources/js/cr/ui/focus_grid.js:164: this.rows[preferredRow].makeActive(true); On 2017/01/06 00:45:29, Dan Beam wrote: > (this.rows[preferredRow] || this.rows[0]).makeActive(true); Neat! 📷
lgtm
The CQ bit was checked by calamity@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: closure_compilation on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)
The CQ bit was checked by calamity@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org Link to the patchset: https://codereview.chromium.org/2618633002/#ps40001 (title: "fix_closure")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483678432840790,
"parent_rev": "34e9fa074784389762eab891850c78371622d216", "commit_rev":
"a4ecd5b7dee7b4ea7444dee445f9757a34d43564"}
Message was sent while issue was closed.
Description was changed from ========== [MD History] Make first URL on Synced Device page instead of menu button. BUG=653398 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [MD History] Make first URL on Synced Device page instead of menu button. BUG=653398 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2618633002 Cr-Commit-Position: refs/heads/master@{#441884} Committed: https://chromium.googlesource.com/chromium/src/+/a4ecd5b7dee7b4ea7444dee445f9... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/a4ecd5b7dee7b4ea7444dee445f9...
Message was sent while issue was closed.
https://codereview.chromium.org/2618633002/diff/40001/chrome/browser/resource... File chrome/browser/resources/md_history/lazy_load.crisper.js (right): https://codereview.chromium.org/2618633002/diff/40001/chrome/browser/resource... chrome/browser/resources/md_history/lazy_load.crisper.js:9: cr.define("cr.ui",function(){function FocusGrid(){this.rows=[]}FocusGrid.prototype={ignoreFocusChange_:false,onFocus:function(row,e){if(this.ignoreFocusChange_)this.ignoreFocusChange_=false;else this.lastFocused_=e.currentTarget;this.rows.forEach(function(r){r.makeActive(r==row)})},onKeydown:function(row,e){var rowIndex=this.rows.indexOf(row);assert(rowIndex>=0);var newRow=-1;if(e.key=="ArrowUp")newRow=rowIndex-1;else if(e.key=="ArrowDown")newRow=rowIndex+1;else if(e.key=="PageUp")newRow=0;else if(e.key=="PageDown")newRow=this.rows.length-1;var rowToFocus=this.rows[newRow];if(rowToFocus){this.ignoreFocusChange_=true;rowToFocus.getEquivalentElement(this.lastFocused_).focus();e.preventDefault();return true}return false},destroy:function(){this.rows.forEach(function(row){row.destroy()});this.rows.length=0},getRowIndexForTarget:function(target){for(var i=0;i<this.rows.length;++i){if(this.rows[i].getElements().indexOf(target)>=0)return i}return-1},getRowForRoot:function(root){for(var i=0;i<this.rows.length;++i){if(this.rows[i].root==root)return this.rows[i]}return null},addRow:function(row){this.addRowBefore(row,null)},addRowBefore:function(row,nextRow){row.delegate=row.delegate||this;var nextRowIndex=nextRow?this.rows.indexOf(nextRow):-1;if(nextRowIndex==-1)this.rows.push(row);else this.rows.splice(nextRowIndex,0,row)},removeRow:function(row){var nextRowIndex=row?this.rows.indexOf(row):-1;if(nextRowIndex>-1)this.rows.splice(nextRowIndex,1)},ensureRowActive:function(preferredRow){if(this.rows.length==0)return;for(var i=0;i<this.rows.length;++i){if(this.rows[i].isActive())return}if(!preferredRow||preferredRow>=this.rows.length)preferredRow=0;this.rows[preferredRow].makeActive(true)}};return{FocusGrid:FocusGrid}});Polymer.PaperButtonBehaviorImpl={properties:{elevation:{type:Number,reflectToAttribute:true,readOnly:true}},observers:["_calculateElevation(focused, disabled, active, pressed, receivedFocusFromKeyboard)","_computeKeyboardClass(receivedFocusFromKeyboard)"],hostAttributes:{role:"button",tabindex:"0",animated:true},_calculateElevation:function(){var e=1;if(this.disabled){e=0}else if(this.active||this.pressed){e=4}else if(this.receivedFocusFromKeyboard){e=3}this._setElevation(e)},_computeKeyboardClass:function(receivedFocusFromKeyboard){this.toggleClass("keyboard-focus",receivedFocusFromKeyboard)},_spaceKeyDownHandler:function(event){Polymer.IronButtonStateImpl._spaceKeyDownHandler.call(this,event);if(this.hasRipple()&&this.getRipple().ripples.length<1){this._ripple.uiDownAction()}},_spaceKeyUpHandler:function(event){Polymer.IronButtonStateImpl._spaceKeyUpHandler.call(this,event);if(this.hasRipple()){this._ripple.uiUpAction()}}};Polymer.PaperButtonBehavior=[Polymer.IronButtonState,Polymer.IronControlState,Polymer.PaperRippleBehavior,Polymer.PaperButtonBehaviorImpl];Polymer({is:"paper-button",behaviors:[Polymer.PaperButtonBehavior],properties:{raised:{type:Boolean,reflectToAttribute:true,value:false,observer:"_calculateElevation"}},_calculateElevation:function(){if(!this.raised){this._setElevation(0)}else{Polymer.PaperButtonBehaviorImpl._calculateElevation.apply(this)}}});Polymer.PaperItemBehaviorImpl={hostAttributes:{role:"option",tabindex:"0"}};Polymer.PaperItemBehavior=[Polymer.IronButtonState,Polymer.IronControlState,Polymer.PaperItemBehaviorImpl];Polymer({is:"paper-item",behaviors:[Polymer.PaperItemBehavior]}); all those fancy presubmits ... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
