From 71fec37579c97922276b1efc286b944bb263bf29 Mon Sep 17 00:00:00 2001 From: Aaron Arcos Date: Tue, 8 Apr 2014 16:01:02 -0700 Subject: [PATCH] Fix issues with size menus after oojs-ui update These should fix all the issues we have now with the size menus. Change-Id: Ief2e5e653330b01c9787e04a10b1b2c425da705e Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/407 --- resources/mmv/ui/mmv.ui.reuse.download.js | 2 +- resources/mmv/ui/mmv.ui.reuse.embed.js | 19 ++++++++++++------- resources/mmv/ui/mmv.ui.reuse.utils.js | 2 +- tests/qunit/mmv/ui/mmv.ui.reuse.embed.test.js | 12 ++++++------ 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/resources/mmv/ui/mmv.ui.reuse.download.js b/resources/mmv/ui/mmv.ui.reuse.download.js index a6dc03bce..bc468cd1a 100644 --- a/resources/mmv/ui/mmv.ui.reuse.download.js +++ b/resources/mmv/ui/mmv.ui.reuse.download.js @@ -44,7 +44,7 @@ * Default item for the size menu. * @property {OO.ui.MenuItemWidget} */ - this.defaultItem = this.downloadSizeMenu.getMenu().getFirstSelectableItem(); + this.defaultItem = this.downloadSizeMenu.getMenu().getSelectedItem(); } oo.inheritClass( Download, mw.mmv.ui.reuse.Tab ); DP = Download.prototype; diff --git a/resources/mmv/ui/mmv.ui.reuse.embed.js b/resources/mmv/ui/mmv.ui.reuse.embed.js index bcb056bba..69eb26a7c 100644 --- a/resources/mmv/ui/mmv.ui.reuse.embed.js +++ b/resources/mmv/ui/mmv.ui.reuse.embed.js @@ -70,13 +70,13 @@ * Default item for the html size menu. * @property {OO.ui.MenuItemWidget} */ - this.defaultHtmlItem = this.embedSizeSwitchHtml.getMenu().getFirstSelectableItem(); + this.defaultHtmlItem = this.embedSizeSwitchHtml.getMenu().getSelectedItem(); /** * Default item for the wikitext size menu. * @property {OO.ui.MenuItemWidget} */ - this.defaultWikitextItem = this.embedSizeSwitchWikitext.getMenu().getFirstSelectableItem(); + this.defaultWikitextItem = this.embedSizeSwitchWikitext.getMenu().getSelectedItem(); /** * Currently selected size menu. @@ -211,8 +211,8 @@ this.embedSwitch.on( 'select', $.proxy( embed.handleTypeSwitch, embed ) ); // Register handlers for switching between file sizes - this.embedSizeSwitchHtml.getMenu().on( 'select', $.proxy( this.handleSizeSwitch, this ) ); - this.embedSizeSwitchWikitext.getMenu().on( 'select', $.proxy( this.handleSizeSwitch, this ) ); + this.embedSizeSwitchHtml.getMenu().on( 'choose', $.proxy( this.handleSizeSwitch, this ) ); + this.embedSizeSwitchWikitext.getMenu().on( 'choose', $.proxy( this.handleSizeSwitch, this ) ); }; /** @@ -224,8 +224,9 @@ this.embedTextHtml.offDOMEvent( 'focus mousedown click' ); this.embedTextWikitext.offDOMEvent( 'focus mousedown click' ); this.embedSwitch.off( 'select' ); - this.embedSizeSwitchHtml.getMenu().off( 'select' ); - this.embedSizeSwitchWikitext.getMenu().off( 'select' ); + // the noop is needed for some tests which call unattach before calling attach. + this.embedSizeSwitchHtml.getMenu().off( 'choose' ); + this.embedSizeSwitchWikitext.getMenu().off( 'choose' ); }; /** @@ -282,7 +283,11 @@ * Reset current menu selection to default item. */ EP.resetCurrentSizeMenuToDefault = function () { - this.currentSizeMenu.selectItem( this.currentDefaultItem ); + this.currentSizeMenu.chooseItem( this.currentDefaultItem ); + // Force select logic to update the selected item bar, otherwise we end up + // with the wrong label. This is implementation dependent and maybe it should + // be done via a to flag to OO.ui.SelectWidget.prototype.chooseItem()? + this.currentSizeMenu.emit( 'select', this.currentDefaultItem ); }; /** diff --git a/resources/mmv/ui/mmv.ui.reuse.utils.js b/resources/mmv/ui/mmv.ui.reuse.utils.js index a9f359f18..e417efd27 100644 --- a/resources/mmv/ui/mmv.ui.reuse.utils.js +++ b/resources/mmv/ui/mmv.ui.reuse.utils.js @@ -66,7 +66,7 @@ } menu.getMenu().addItems( items ); - menu.getMenu().selectItem( choices[ def ] ); + menu.getMenu().chooseItem( choices[ def ] ); return menu; }; diff --git a/tests/qunit/mmv/ui/mmv.ui.reuse.embed.test.js b/tests/qunit/mmv/ui/mmv.ui.reuse.embed.test.js index 7950cb30b..29c7ebf5b 100644 --- a/tests/qunit/mmv/ui/mmv.ui.reuse.embed.test.js +++ b/tests/qunit/mmv/ui/mmv.ui.reuse.embed.test.js @@ -299,9 +299,9 @@ embed.embedTextWikitext.$element.focus(); embed.embedSwitch.emit( 'select' ); embed.embedSizeSwitchHtml.getMenu().emit( - 'select', embed.embedSizeSwitchHtml.getMenu().getSelectedItem() ); + 'choose', embed.embedSizeSwitchHtml.getMenu().getSelectedItem() ); embed.embedSizeSwitchWikitext.getMenu().emit( - 'select', embed.embedSizeSwitchWikitext.getMenu().getSelectedItem() ); + 'choose', embed.embedSizeSwitchWikitext.getMenu().getSelectedItem() ); embed.selectAllOnEvent = function() { assert.ok( true, 'selectAllOnEvent was called.' ); @@ -320,9 +320,9 @@ embed.embedTextWikitext.$element.focus(); embed.embedSwitch.emit( 'select' ); embed.embedSizeSwitchHtml.getMenu().emit( - 'select', embed.embedSizeSwitchHtml.getMenu().getSelectedItem() ); + 'choose', embed.embedSizeSwitchHtml.getMenu().getSelectedItem() ); embed.embedSizeSwitchWikitext.getMenu().emit( - 'select', embed.embedSizeSwitchWikitext.getMenu().getSelectedItem() ); + 'choose', embed.embedSizeSwitchWikitext.getMenu().getSelectedItem() ); // Test the unattach part embed.selectAllOnEvent = function() { @@ -342,9 +342,9 @@ embed.embedTextWikitext.$element.focus(); embed.embedSwitch.emit( 'select' ); embed.embedSizeSwitchHtml.getMenu().emit( - 'select', embed.embedSizeSwitchHtml.getMenu().getSelectedItem() ); + 'choose', embed.embedSizeSwitchHtml.getMenu().getSelectedItem() ); embed.embedSizeSwitchWikitext.getMenu().emit( - 'select', embed.embedSizeSwitchWikitext.getMenu().getSelectedItem() ); + 'choose', embed.embedSizeSwitchWikitext.getMenu().getSelectedItem() ); } ); QUnit.test( 'handleTypeSwitch():', 6, function ( assert ) {