jenkins-bot has submitted this change and it was merged.
Change subject: Fix the logic of apply and cancel actions
......................................................................
Fix the logic of apply and cancel actions
* Refactored the Cancel button handler code to cancel method in
display settings and input settings.
* When the user makes changes in multiple modules and clicks the Cancel
button or closes the language settings after that, cancel the changes in
all the modules. See bug 50564.
* The Apply button was always active in input methods module. Fixed the
logic for that.
* Renamed the enableApplyButton method to markDirty in both modules.
* Introduced isDirty attribute to the modules for optimizing the Cancel
method to avoid unnecessary restore actions.
* More minor cleanup and documentation.
Bug: 50564
Change-Id: I71f527bfb7dd7f6724e4365371ac3e4fc0723ed6
---
M resources/js/ext.uls.displaysettings.js
M resources/js/ext.uls.inputsettings.js
M resources/js/ext.uls.languagesettings.js
3 files changed, 107 insertions(+), 51 deletions(-)
Approvals:
Amire80: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/js/ext.uls.displaysettings.js
b/resources/js/ext.uls.displaysettings.js
index f538912..79caa42 100644
--- a/resources/js/ext.uls.displaysettings.js
+++ b/resources/js/ext.uls.displaysettings.js
@@ -119,6 +119,8 @@
this.i18n();
this.$webfonts.refresh();
this.listen();
+ this.dirty = false;
+ this.savedRegistry = $.extend( true, {},
mw.webfonts.preferences );
},
/**
@@ -196,7 +198,7 @@
function buttonHandler( button ) {
return function () {
- displaySettings.enableApplyButton();
+ displaySettings.markDirty();
displaySettings.uiLanguage =
button.data( 'language' ) || displaySettings.uiLanguage;
$( 'div.uls-ui-languages button.button'
).removeClass( 'down' );
button.addClass( 'down' );
@@ -291,7 +293,7 @@
}
},
onSelect: function ( langCode ) {
- displaySettings.enableApplyButton();
+ displaySettings.markDirty();
displaySettings.uiLanguage = langCode;
displaySettings.$parent.show();
displaySettings.prepareUIFonts();
@@ -444,11 +446,16 @@
},
/**
- * Enable the apply button.
+ * Mark dirty, there are unsaved changes. Enable the apply
button.
* Useful in many places when something changes.
*/
- enableApplyButton: function () {
+ markDirty: function () {
+ this.dirty = true;
this.$template.find( '#uls-displaysettings-apply'
).removeAttr( 'disabled' );
+ },
+
+ disableApplyButton: function () {
+ this.$template.find( '#uls-displaysettings-apply'
).prop( 'disabled', true );
},
/**
@@ -458,8 +465,6 @@
var displaySettings = this,
$contentFontSelector = this.$template.find(
'#content-font-selector' ),
$uiFontSelector = this.$template.find(
'#ui-font-selector' ),
- oldUIFont = $uiFontSelector.find(
'option:selected' ).val(),
- oldContentFont = $contentFontSelector.find(
'option:selected' ).val(),
$tabButtons = displaySettings.$template.find(
'.uls-display-settings-tab-switcher button' );
// TODO all these repeated selectors can be placed in
object constructor.
@@ -469,30 +474,14 @@
} );
displaySettings.$template.find(
'button.uls-display-settings-cancel' ).on( 'click', function () {
- mw.webfonts.preferences.setFont(
displaySettings.contentLanguage, oldContentFont );
- mw.webfonts.preferences.setFont(
displaySettings.uiLanguage, oldUIFont );
-
- if ( displaySettings.$webfonts ) {
- displaySettings.$webfonts.refresh();
- }
-
- displaySettings.$template.find(
'div.uls-ui-languages button.button' ).each( function () {
- var $button = $( this );
-
- if ( $button.attr( 'lang' ) ===
displaySettings.contentLanguage ) {
- $button.addClass( 'down' );
- } else {
- $button.removeClass( 'down' );
- }
- } );
+ displaySettings.cancel();
displaySettings.prepareUIFonts();
displaySettings.prepareContentFonts();
displaySettings.close();
} );
$uiFontSelector.on( 'change', function () {
- displaySettings.enableApplyButton();
- oldUIFont = mw.webfonts.preferences.getFont(
displaySettings.uiLanguage );
+ displaySettings.markDirty();
mw.webfonts.preferences.setFont(
displaySettings.uiLanguage,
$( this ).find( 'option:selected'
).val()
);
@@ -500,8 +489,7 @@
} );
$contentFontSelector.on( 'change', function () {
- displaySettings.enableApplyButton();
- oldContentFont =
mw.webfonts.preferences.getFont( displaySettings.contentLanguage );
+ displaySettings.markDirty();
mw.webfonts.preferences.setFont(
displaySettings.contentLanguage,
$( this ).find( 'option:selected'
).val()
);
@@ -583,6 +571,46 @@
mw.webfonts.preferences.save( function ( result ) {
// closure for not losing the scope
displaySettings.onSave( result );
+ displaySettings.dirty = false;
+ // Update the back-up preferences for the case
of canceling
+ displaySettings.savedRegistry = $.extend( true,
{}, mw.webfonts.preferences );
+ } );
+ },
+
+ /**
+ * Cancel the changes done by user for display settings
+ */
+ cancel: function () {
+ var displaySettings = this;
+
+ if ( !displaySettings.dirty ) {
+ // Nothing changed
+ return;
+ }
+
+ // Reload preferences
+ mw.webfonts.preferences = $.extend( true, {},
displaySettings.savedRegistry );
+ if ( displaySettings.$webfonts ) {
+ displaySettings.$webfonts.refresh();
+ }
+
+ // Clear the dirty bit
+ displaySettings.dirty = false;
+ displaySettings.disableApplyButton();
+
+ // Restore content and UI language
+ displaySettings.uiLanguage =
displaySettings.getUILanguage();
+ displaySettings.contentLanguage =
displaySettings.getContentLanguage();
+
+ // Restore the visual state of selected language button
+ displaySettings.$template.find( 'div.uls-ui-languages
button.button' ).each( function () {
+ var $button = $( this );
+
+ if ( $button.attr( 'lang' ) ===
displaySettings.uiLanguage ) {
+ $button.addClass( 'down' );
+ } else {
+ $button.removeClass( 'down' );
+ }
} );
}
};
diff --git a/resources/js/ext.uls.inputsettings.js
b/resources/js/ext.uls.inputsettings.js
index 5a1a90e..ae0cc72 100644
--- a/resources/js/ext.uls.inputsettings.js
+++ b/resources/js/ext.uls.inputsettings.js
@@ -75,6 +75,7 @@
this.contentLanguage = this.getContentLanguage();
this.$imes = null;
this.$parent = $parent;
+ this.dirty = false;
this.savedRegistry = $.extend( true, {},
$.ime.preferences.registry );
}
@@ -109,10 +110,11 @@
},
/**
- * Enable the apply button.
+ * Mark dirty, there are unsaved changes. Enable the apply
button.
* Useful in many places when something changes.
*/
- enableApplyButton: function () {
+ markDirty: function () {
+ this.dirty = true;
this.$template.find( 'button.uls-input-settings-apply'
).prop( 'disabled', false );
},
@@ -279,8 +281,10 @@
return function () {
var language = button.data( 'language'
);
- inputSettings.enableApplyButton();
- $.ime.preferences.setLanguage( language
);
+ if ( language !==
$.ime.preferences.getLanguage() ) {
+ inputSettings.markDirty();
+ $.ime.preferences.setLanguage(
language );
+ }
// Mark the button selected
$( '.uls-ui-languages .button'
).removeClass( 'down' );
button.addClass( 'down' );
@@ -375,7 +379,7 @@
}
},
onSelect: function ( langCode ) {
- inputSettings.enableApplyButton();
+ inputSettings.markDirty();
$.ime.preferences.setLanguage( langCode
);
inputSettings.$parent.show();
inputSettings.prepareLanguages();
@@ -447,32 +451,20 @@
} );
inputSettings.$template.find(
'button.uls-input-settings-cancel' ).on( 'click', function () {
- inputSettings.disableApplyButton();
-
- inputSettings.close();
-
- // Reload preferences
- $.ime.preferences.registry = $.extend( true,
{}, inputSettings.savedRegistry );
-
- // Restore the state of IME
- if ( $.ime.preferences.isEnabled() ) {
- mw.ime.setup();
- } else {
- mw.ime.disable();
- }
-
+ inputSettings.cancel();
// Redraw the panel according to the state
inputSettings.render();
+ inputSettings.close();
} );
$imeListContainer.on( 'change',
'input:radio[name=ime]:checked', function () {
- inputSettings.enableApplyButton();
+ inputSettings.markDirty();
$.ime.preferences.setIM( $( this ).val() );
} );
inputSettings.$template.find(
'button.uls-input-toggle-button' )
.on( 'click', function () {
- inputSettings.enableApplyButton();
+ inputSettings.markDirty();
if ( $.ime.preferences.isEnabled() ) {
inputSettings.disableInputTools();
@@ -540,10 +532,31 @@
$.ime.preferences.save( function ( result ) {
// closure for not losing the scope
inputSettings.onSave( result );
+ inputSettings.dirty = false;
+ // Update the back-up preferences for the case
of canceling
+ inputSettings.savedRegistry = $.extend( true,
{}, $.ime.preferences.registry );
} );
+ },
- // Update the back-up preferences for the case of
canceling
- this.savedRegistry = $.extend( true, {},
$.ime.preferences.registry );
+ /**
+ * Cancel the changes done by user for input settings
+ */
+ cancel: function () {
+ if ( !this.dirty ) {
+ return;
+ }
+ this.dirty = false;
+ this.disableApplyButton();
+
+ // Reload preferences
+ $.ime.preferences.registry = $.extend( true, {},
this.savedRegistry );
+
+ // Restore the state of IME
+ if ( $.ime.preferences.isEnabled() ) {
+ mw.ime.setup();
+ } else {
+ mw.ime.disable();
+ }
}
};
diff --git a/resources/js/ext.uls.languagesettings.js
b/resources/js/ext.uls.languagesettings.js
index d774031..5c1848a 100644
--- a/resources/js/ext.uls.languagesettings.js
+++ b/resources/js/ext.uls.languagesettings.js
@@ -49,6 +49,7 @@
this.initialized = false;
this.left = this.options.left;
this.top = this.options.top;
+ this.modules = {},
this.$settingsPanel = this.$window.find(
'#languagesettings-settings-panel' );
this.init();
this.listen();
@@ -72,8 +73,8 @@
$( 'html' ).click( $.proxy( this.hide, this ) );
// ... but when clicked on window do not hide.
- this.$window.on( 'click', function ( e ) {
- e.stopPropagation();
+ this.$window.on( 'click', function ( event ) {
+ event.stopPropagation();
} );
},
@@ -141,6 +142,7 @@
module.render();
$settingsLink.addClass( 'active' );
}
+ this.modules[moduleName] = module;
},
position: function () {
@@ -204,11 +206,24 @@
* call onClose if defined from the previous context.
*/
close: function () {
+ if ( !this.shown ) {
+ return;
+ }
+
this.hide();
+ // optional callback
if ( this.options.onClose ) {
this.options.onClose();
}
+
+ // We are closing language settings. That also means we
are cancelling
+ // any changes the user did, but not saved, in all
registered modules.
+ $.each( this.modules, function( id, module ) {
+ // Modules should make sure to return early if
no changes were made
+ // They can use some kind of 'dirty bits' to
implement this.
+ module.cancel();
+ } );
},
click: function ( e ) {
--
To view, visit https://gerrit.wikimedia.org/r/75096
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I71f527bfb7dd7f6724e4365371ac3e4fc0723ed6
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/UniversalLanguageSelector
Gerrit-Branch: master
Gerrit-Owner: Santhosh <santhosh.thottin...@gmail.com>
Gerrit-Reviewer: Amire80 <amir.ahar...@mail.huji.ac.il>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
