diff --git a/resources/css/ext.uls.languagesettings.css b/resources/css/ext.uls.languagesettings.css index 7c5e61f5..b7a1b94c 100644 --- a/resources/css/ext.uls.languagesettings.css +++ b/resources/css/ext.uls.languagesettings.css @@ -92,7 +92,7 @@ background-color: #F5F5F5; } -#languagesettings-settings-panel .language-settings-buttons { +.language-settings-buttons { border-top: 1px solid #F0F0F0; margin-top: 25px; padding: 15px; diff --git a/resources/js/ext.uls.displaysettings.js b/resources/js/ext.uls.displaysettings.js index 3b7e6583..cfcbe45e 100644 --- a/resources/js/ext.uls.displaysettings.js +++ b/resources/js/ext.uls.displaysettings.js @@ -78,19 +78,9 @@ + '' // End font selectors - + '' // End font settings section + + ''; // End font settings section - // Separator - + '
' - // Apply and Cancel buttons - + '
' - + '
' - + '' - + '' - + '
' - + '
' - + ''; function DisplaySettings( $parent ) { this.name = $.i18n( 'ext-uls-display-settings-title-short' ); @@ -452,8 +442,7 @@ * i18n this settings panel */ i18n: function () { - this.$template.i18n(); - + this.$parent.i18n(); this.$template.find( '#ui-font-selector-label strong' ) .text( $.i18n( 'ext-uls-webfonts-select-for', $.uls.data.getAutonym( this.uiLanguage ) ) ); this.$template.find( '#content-font-selector-label strong' ) @@ -485,11 +474,11 @@ */ markDirty: function () { this.dirty = true; - this.$template.find( '#uls-displaysettings-apply' ).removeAttr( 'disabled' ); + this.$parent.$window.find( 'button.uls-settings-apply' ).removeAttr( 'disabled' ); }, disableApplyButton: function () { - this.$template.find( '#uls-displaysettings-apply' ).prop( 'disabled', true ); + this.$parent.$window.find( 'button.uls-settings-apply' ).prop( 'disabled', true ); }, /** @@ -503,15 +492,6 @@ // TODO all these repeated selectors can be placed in object constructor. - displaySettings.$template.find( '#uls-displaysettings-apply' ).on( 'click', function () { - displaySettings.apply(); - } ); - - displaySettings.$template.find( 'button.uls-display-settings-cancel' ).on( 'click', function () { - displaySettings.cancel(); - displaySettings.close(); - } ); - $uiFontSelector.on( 'change', function () { displaySettings.markDirty(); mw.webfonts.preferences.setFont( displaySettings.uiLanguage, @@ -551,7 +531,6 @@ } ); - mw.hook( 'mw.uls.settings.cancel' ).add( $.proxy( this.cancel, this ) ); }, /** @@ -624,6 +603,7 @@ */ cancel: function () { if ( !this.dirty ) { + this.close(); return; } // Reload preferences @@ -637,6 +617,7 @@ // Restore content and UI language this.uiLanguage = this.getUILanguage(); this.contentLanguage = this.getContentLanguage(); + this.close(); } }; diff --git a/resources/js/ext.uls.inputsettings.js b/resources/js/ext.uls.inputsettings.js index 81bafbc8..c5f8808f 100644 --- a/resources/js/ext.uls.inputsettings.js +++ b/resources/js/ext.uls.inputsettings.js @@ -53,18 +53,6 @@ + '
' + '' + '
' - + '' - - // Separator - + '
' - - // Apply and Cancel buttons - + '
' - + '
' - + '' - + '' - + '
' - + '
' + ''; function InputSettings( $parent ) { @@ -76,6 +64,8 @@ this.$imes = null; this.$parent = $parent; this.savedRegistry = $.extend( true, {}, $.ime.preferences.registry ); + // ime system is lazy loaded, make sure it is initialized + mw.ime.init(); } InputSettings.prototype = { @@ -92,8 +82,6 @@ this.$imes = $( 'body' ).data( 'ime' ); this.$parent.$settingsPanel.append( this.$template ); $enabledOnly = this.$template.find( '.enabled-only' ); - // ime system is lazy loaded, make sure it is initialized - mw.ime.init(); if ( $.ime.preferences.isEnabled() ) { $enabledOnly.removeClass( 'hide' ); } else { @@ -103,8 +91,7 @@ this.prepareLanguages(); this.prepareToggleButton(); - this.$template.i18n(); - this.disableApplyButton(); + this.$parent.i18n(); $( 'body' ).data( 'webfonts' ).refresh(); this.listen(); }, @@ -115,11 +102,11 @@ */ markDirty: function () { this.dirty = true; - this.$template.find( 'button.uls-input-settings-apply' ).prop( 'disabled', false ); + this.$parent.$window.find( 'button.uls-settings-apply' ).prop( 'disabled', false ); }, disableApplyButton: function () { - this.$template.find( 'button.uls-input-settings-apply' ).prop( 'disabled', true ); + this.$parent.$window.find( 'button.uls-settings-apply' ).prop( 'disabled', true ); }, prepareInputmethods: function ( language ) { @@ -446,16 +433,6 @@ $imeListContainer = this.$template.find( '.uls-input-settings-inputmethods-list' ); - // Apply and close buttons - inputSettings.$template.find( 'button.uls-input-settings-apply' ).on( 'click', function () { - inputSettings.apply(); - } ); - - inputSettings.$template.find( 'button.uls-input-settings-cancel' ).on( 'click', function () { - inputSettings.cancel(); - inputSettings.close(); - } ); - $imeListContainer.on( 'change', 'input:radio[name=ime]:checked', function () { inputSettings.markDirty(); $.ime.preferences.setIM( $( this ).val() ); @@ -472,7 +449,6 @@ } } ); - mw.hook( 'mw.uls.settings.cancel' ).add( $.proxy( this.cancel, this ) ); }, /** @@ -564,6 +540,7 @@ */ cancel: function () { if ( !this.dirty ) { + this.close(); return; } // Reload preferences @@ -576,6 +553,7 @@ } else { mw.ime.disable(); } + this.close(); } }; diff --git a/resources/js/ext.uls.languagesettings.js b/resources/js/ext.uls.languagesettings.js index 02bfafb2..6e408154 100644 --- a/resources/js/ext.uls.languagesettings.js +++ b/resources/js/ext.uls.languagesettings.js @@ -20,7 +20,7 @@ ( function ( $, mw ) { 'use strict'; - var closeRow, settingsMenu, settingsPanel, windowTemplate, panelsRow; + var closeRow, settingsMenu, settingsPanel, windowTemplate, panelsRow, buttonsRow; closeRow = '
' + '
' + @@ -32,9 +32,19 @@ '
'; settingsPanel = '
' + '
'; + buttonsRow = '
' + + // Apply and Cancel buttons + '
' + + '
' + + '' + + '' + + '
' + + '
' + + ''; panelsRow = '
' + settingsMenu + settingsPanel + + buttonsRow + '
'; windowTemplate = '
' + closeRow + @@ -66,9 +76,11 @@ // Register all event listeners to the ULS language settings here. listen: function () { this.$element.on( 'click', $.proxy( this.click, this ) ); - this.$window.find( '#languagesettings-close' ) - .on( 'click', $.proxy( this.close, this ) ); + this.$window.find( '#languagesettings-close, button.uls-settings-cancel' ) + .on( 'click', $.proxy( mw.hook( 'mw.uls.settings.cancel' ).fire, this ) ); + this.$window.find( 'button.uls-settings-apply' ) + .on( 'click', $.proxy( mw.hook( 'mw.uls.settings.apply' ).fire, this ) ); // Hide the window when clicked outside $( 'html' ).click( $.proxy( this.hide, this ) ); @@ -95,17 +107,18 @@ } // Call render function on the current setting module. - languageSettings.renderModule( moduleName, defaultModule === moduleName ); + languageSettings.initModule( moduleName, defaultModule === moduleName ); } } ); }, /** + * Initialize the module. * Render the link and settings area for a language setting module. - * @param moduleName String Name of the setting module - * @param active boolean Make this module active and show by default + * @param {string} moduleName Name of the setting module + * @param {boolean} active boolean Make this module active and show by default */ - renderModule: function ( moduleName, active ) { + initModule: function ( moduleName, active ) { var $settingsTitle, $settingsText, $settingsLink, languageSettings = this, module = new $.fn.languagesettings.modules[moduleName]( languageSettings ), @@ -132,7 +145,7 @@ var $this = $( this ); $this.data( 'module' ).render(); - // re-position the window and scroll in to view if required. + // Re-position the window and scroll in to view if required. languageSettings.position(); $settingsMenuItems.find( '.menu-section' ).removeClass( 'active' ); $this.addClass( 'active' ); @@ -143,6 +156,10 @@ $settingsLink.addClass( 'active' ); } this.modules[moduleName] = module; + + // Register cancel and apply hooks + mw.hook( 'mw.uls.settings.cancel' ).add( $.proxy( module.cancel, module ) ); + mw.hook( 'mw.uls.settings.apply' ).add( $.proxy( module.apply, module ) ); }, position: function () { @@ -160,6 +177,10 @@ this.$window.scrollIntoView(); }, + i18n: function () { + this.$window.i18n(); + }, + show: function () { if ( !this.initialized ) { this.render(); @@ -167,7 +188,7 @@ } // close model windows close, if they hide on page click $( 'html' ).click(); - this.$window.i18n(); + this.i18n(); this.shown = true; this.$window.show(); @@ -217,7 +238,6 @@ this.options.onClose(); } - mw.hook( 'mw.uls.settings.cancel' ).fire(); }, click: function ( e ) { diff --git a/tests/browser/features/step_definitions/panel_steps.rb b/tests/browser/features/step_definitions/panel_steps.rb index 3caf222d..6737682b 100644 --- a/tests/browser/features/step_definitions/panel_steps.rb +++ b/tests/browser/features/step_definitions/panel_steps.rb @@ -68,10 +68,10 @@ Then(/^the selected (.*?) font must be "(.*?)"$/) do |type, font| end When(/^I apply the changes$/) do - on(ULSPage).panel_button_display_apply_element.click - wait = Selenium::WebDriver::Wait.new(:timeout => 3) - panel = @browser.driver.find_element(:id => 'language-settings-dialog') - wait.until { !panel.displayed? } + on(ULSPage).panel_button_apply_element.click + # Leave a little time for the settings to be saved. The settings window closes + # immediately, so it is not enough to wait for it to disappear. + sleep 2 end Then(/^the (.*) font must be changed to the "(.*?)" font$/) do |type, font| diff --git a/tests/browser/features/step_definitions/uls_cog_sidebar_user_steps.rb b/tests/browser/features/step_definitions/uls_cog_sidebar_user_steps.rb index 5b6d3605..77d525dc 100644 --- a/tests/browser/features/step_definitions/uls_cog_sidebar_user_steps.rb +++ b/tests/browser/features/step_definitions/uls_cog_sidebar_user_steps.rb @@ -27,12 +27,8 @@ Given(/^I navigate to the Language Settings panel$/) do step 'I see the logged in language settings panel' end -When(/^I click Apply Settings$/) do - on(InterlanguagePage).apply_settings_element.click -end - When(/^I click Cancel$/) do - on(InterlanguagePage).cancel_element.click + on(ULSPage).panel_button_cancel_element.click end When(/^I click on the link to select Malayalam$/) do diff --git a/tests/browser/features/support/modules/interlanguage_module.rb b/tests/browser/features/support/modules/interlanguage_module.rb index cd9dc4e1..275f03ca 100644 --- a/tests/browser/features/support/modules/interlanguage_module.rb +++ b/tests/browser/features/support/modules/interlanguage_module.rb @@ -4,10 +4,8 @@ module InterlanguagePageModule include PageObject a(:add_links, id: 'wbc-linkToItem-link') - span(:apply_settings, class: 'uls-settings-trigger') a(:back_to_display, text: 'Back to display settings') a(:back_to_input, text: 'Back to input settings') - button(:cancel, class: 'button uls-display-settings-cancel') span(:cog, class: 'uls-settings-trigger') button(:ellipsis_button, class: 'uls-more-languages button') a(:english_link, text: 'English') diff --git a/tests/browser/features/support/pages/panel_page.rb b/tests/browser/features/support/pages/panel_page.rb index fb159a46..d937b32e 100644 --- a/tests/browser/features/support/pages/panel_page.rb +++ b/tests/browser/features/support/pages/panel_page.rb @@ -10,7 +10,8 @@ class ULSPage button(:panel_language, id: 'uls-display-settings-language-tab') span(:panel_button_close, id: 'languagesettings-close') - button(:panel_button_display_apply, id: 'uls-displaysettings-apply') + button(:panel_button_apply, class: 'uls-settings-apply') + button(:panel_button_cancel, class: 'uls-settings-cancel') button(:panel_disable_input_methods, class: 'uls-input-toggle-button') button(:panel_enable_input_methods, class: 'uls-input-toggle-button') diff --git a/tests/browser/features/uls_triggers.feature b/tests/browser/features/uls_triggers.feature index 9dbb2417..319c803d 100644 --- a/tests/browser/features/uls_triggers.feature +++ b/tests/browser/features/uls_triggers.feature @@ -35,6 +35,3 @@ Feature: ULS trigger in personal toolbar | user status | close method | | logged in | I click X | | logged out | I click Cancel | - # It is weird that this works, since the button is disabled until changes - # have been made - | logged out | I click Apply Settings |