Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Handle form reset in form widgets without introducing a base class #5214

Closed
wants to merge 4 commits into from

4 participants

@gabrielschulhof
Collaborator

No description provided.

@johnbender

The only two thoughts I have are, first, that tacking the extension onto the already crowded global namespace $.mobile is probably a bad idea, and that we should extend the prototype directly. Though I understand why you put it on the global namespace, where else would you put it?

I think it's probably time (it probably was time for this a long time ago) to commit to a project convention for widget extension that isn't the inheritance hierarchy.

Here's my proposal.

First we create top level namespace for common behaviors in jquery.mobile.core.js:

// ...

// namespace to contain extension functions for widgets and other object constructors
$.mobile.behaviors = {};

// ... 

Then for the current common functionality we can create individual behaviors like so:

In widgets/forms/reset.js

define(["jquery", "core"], function( $ ) {
    $.mobile.behaviors.formReset = function( widget ) {
        widget.prototype._formReset(function() {
            var self = this;

            self._on( self.element.closest( "form" ), {
                "reset": function() {
                    setTimeout( $.proxy( self, "_reset" ) );
                }
            });
        });
    };
});

In widgets/forms/selectmenu.js

$.widget( "mobile.selectmenu", $.mobile.widget, {
    _create: function() {
        // ...

        this._formReset();

        // ...
    }
});

$.mobile.behaviors.formReset( $.mobile.selectmenu );
@frequent

how about actions instead of behaviors ?

@johnbender

@frequent it's not necessarily going to be a single action, it could be a collection of behaviors. "Behaviors" is poor anyway, but that's a general software convention (Mixins => behavioral).

I have to fix this up to use $.widget anyhow. See @scottgonzalez's comment

@johnbender

Adjusted per @scottgonzalez's suggestion

define(["jquery", "core"], function( $ ) {
    $.mobile.behaviors.formReset = {
        _formReset: function() {
            var self = this;

            self._on( self.element.closest( "form" ), {
                "reset": function() {
                    setTimeout( $.proxy( self, "_reset" ) );
                }
            });
        }
    };
});

$.widget( "mobile.selectmenu", $.mobile.widget, {
    _create: function() {
        // ...

        this._formReset();

        // ...
    }
});

$.widget( "mobile.selectmenu", $.mobile.behaviors.formReset );
@scottgonzalez

You should very rarely need to capture this...

_formReset: function() {
    this._on( this.element.closest( "form" ), {
        reset: function() {
            this._delay( "_reset" );
        }
    });
}
@johnbender

@frequent

That was a poor explanation. Actions suggests, to me at least, a single function/method. In this case we might have collections of functions or methods that work together to form some common "behavior" - that's the idea at least.

@frequent

@johnbender

I see. I was thinking along, like in a collection of "actions", but general software convention does sound convincing :-)

@gabrielschulhof
Collaborator

Hey! I've re-implemented this with behaviours: #5216

@frequent

@gabrielschulhof: so with this implemented, I could ditch my "manual form widget reset", since JQM will reset selects/sliders et al to their intial values?

One thing I'm starting to like lately is adding a data-persist="true" attribute on form elements, which then are not reset when I'm running my form cleanup. Anyway, can't wait to use this :+1:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
1  js/index.php
@@ -42,6 +42,7 @@
'widgets/listview.filter.js',
'widgets/listview.autodividers.js',
'jquery.mobile.nojs.js',
+ 'widgets/forms/form.resetsetup.js',
'widgets/forms/checkboxradio.js',
'widgets/forms/button.js',
'widgets/forms/slider.js',
View
7 js/widgets/forms/checkboxradio.js
@@ -9,7 +9,7 @@
//>>css.structure: ../css/structure/jquery.mobile.forms.checkboxradio.css
//>>css.theme: ../css/themes/default/jquery.mobile.theme.css
-define( [ "jquery", "../../jquery.mobile.core", "../../jquery.mobile.widget", "../../jquery.mobile.buttonMarkup" ], function( $ ) {
+define( [ "jquery", "../../jquery.mobile.core", "../../jquery.mobile.widget", "../../jquery.mobile.buttonMarkup", "./form.resetsetup" ], function( $ ) {
//>>excludeEnd("jqmBuildExclude");
(function( $, undefined ) {
@@ -138,6 +138,7 @@ $.widget( "mobile.checkboxradio", $.mobile.widget, {
}
});
+ $.mobile._formResetSetup( this );
this.refresh();
},
@@ -170,6 +171,10 @@ $.widget( "mobile.checkboxradio", $.mobile.widget, {
.checkboxradio( "refresh" );
},
+ _reset: function() {
+ this.refresh();
+ },
+
refresh: function() {
var input = this.element[0],
label = this.label,
View
22 js/widgets/forms/form.resetsetup.js
@@ -0,0 +1,22 @@
+//>>excludeStart("jqmBuildExclude", pragmas.jqmBuildExclude);
+//>>description: form reset support for form widgets
+//>>label: formReset
+//>>group: Forms
+
+define( [ "jquery", "../../jquery.mobile.core" ], function( $ ) {
+//>>excludeEnd("jqmBuildExclude");
+(function( $, undefined ) {
+
+ function addResetTimeout() {
+ setTimeout( $.proxy( this, "_reset" ) );
+ }
+
+ $.mobile._formResetSetup = function( widget ) {
+ widget._on( widget.element.closest( "form" ), {
+ "reset": $.proxy( addResetTimeout, widget )
+ });
+ };
+})( jQuery );
+//>>excludeStart("jqmBuildExclude", pragmas.jqmBuildExclude);
+});
+//>>excludeEnd("jqmBuildExclude");
View
7 js/widgets/forms/select.js
@@ -5,7 +5,7 @@
//>>css.structure: ../css/structure/jquery.mobile.forms.select.css
//>>css.theme: ../css/themes/default/jquery.mobile.theme.css
-define( [ "jquery", "../../jquery.mobile.core", "../../jquery.mobile.widget", "../../jquery.mobile.buttonMarkup", "../../jquery.mobile.zoom" ], function( $ ) {
+define( [ "jquery", "../../jquery.mobile.core", "../../jquery.mobile.widget", "../../jquery.mobile.buttonMarkup", "../../jquery.mobile.zoom", "./form.resetsetup" ], function( $ ) {
//>>excludeEnd("jqmBuildExclude");
(function( $, undefined ) {
@@ -141,6 +141,7 @@ $.widget( "mobile.selectmenu", $.mobile.widget, {
self.refresh();
});
+ $.mobile._formResetSetup( this );
this.build();
},
@@ -231,6 +232,10 @@ $.widget( "mobile.selectmenu", $.mobile.widget, {
}
},
+ _reset: function() {
+ this.refresh();
+ },
+
refresh: function() {
this.setButtonText();
this.setButtonCount();
View
8 js/widgets/forms/slider.js
@@ -5,7 +5,7 @@
//>>css.structure: ../css/structure/jquery.mobile.forms.slider.css
//>>css.theme: ../css/themes/default/jquery.mobile.theme.css
-define( [ "jquery", "../../jquery.mobile.core", "../../jquery.mobile.widget", "./textinput", "../../jquery.mobile.buttonMarkup" ], function( $ ) {
+define( [ "jquery", "../../jquery.mobile.core", "../../jquery.mobile.widget", "./textinput", "../../jquery.mobile.buttonMarkup", "./form.resetsetup" ], function( $ ) {
//>>excludeEnd("jqmBuildExclude");
(function( $, undefined ) {
@@ -178,6 +178,8 @@ $.widget( "mobile.slider", $.mobile.widget, {
this.handle.bind( "vclick", false );
+ $.mobile._formResetSetup( this );
+
this.refresh( undefined, undefined, true );
},
@@ -341,6 +343,10 @@ $.widget( "mobile.slider", $.mobile.widget, {
return this.isToggleSwitch ? this.element[0].selectedIndex : parseFloat( this.element.val() ) ;
},
+ _reset: function() {
+ this.refresh( undefined, false, true );
+ },
+
refresh: function( val, isfromControl, preventInputUpdate ) {
// NOTE: we don't return here because we want to support programmatic
Something went wrong with that request. Please try again.