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
This commit is contained in:
Santhosh Thottingal
2013-07-22 15:55:41 +05:30
committed by Amir E. Aharoni
parent 2159822145
commit f11ef736a4
3 changed files with 107 additions and 51 deletions

View File

@@ -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,13 +446,18 @@
},
/**
* 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 );
},
/**
* Register general event listeners
*/
@@ -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' );
}
} );
}
};

View File

@@ -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();
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
this.savedRegistry = $.extend( true, {}, $.ime.preferences.registry );
inputSettings.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();
}
}
};

View File

@@ -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 ) {