Skip to content
This repository was archived by the owner on Oct 8, 2021. It is now read-only.

Bugfix for #3288 #3290

Closed
wants to merge 5 commits into from
Closed

Bugfix for #3288 #3290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
59abc6f
slider control improvements
Dec 16, 2011
83765f0
checkboxradio control improvements
Dec 16, 2011
ca2ada8
custom selectmenu: markup creation loop improvements
Dec 16, 2011
9afaf8b
custom selectmenu: dialog page creation improvements.
Dec 16, 2011
8312e62
Fixes #3288 WP7 border-radius CSS performance issue
Dec 16, 2011
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions js/jquery.mobile.dialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,25 @@ $.widget( "mobile.dialog", $.mobile.widget, {
_create: function() {
var self = this,
$el = this.element,
headerCloseButton = $( "<a href='#' data-" + $.mobile.ns + "icon='delete' data-" + $.mobile.ns + "iconpos='notext'>"+ this.options.closeBtnText + "</a>" );
headerCloseButton = $( "<a href='#' data-" + $.mobile.ns + "icon='delete' data-" + $.mobile.ns + "iconpos='notext'>"+ this.options.closeBtnText + "</a>" ),
hasList = ($el.find('ul[data-role="listview"]').length>0)?true:false;

$el.addClass( "ui-overlay-" + this.options.overlayTheme );

// Class the markup for dialog styling
// Set aria role
$el.attr( "role", "dialog" )
var footer = $el.attr( "role", "dialog" )
.addClass( "ui-dialog" )
.find( ":jqmData(role='header')" )
.addClass( "ui-corner-top ui-overlay-shadow" )
.prepend( headerCloseButton )
.end()
.find( ":jqmData(role='content'),:jqmData(role='footer')" )
.addClass( "ui-overlay-shadow" )
.last()
.addClass( "ui-corner-bottom" );
.addClass( "ui-overlay-shadow" );

if(!hasList){
footer.last().addClass( "ui-corner-bottom" );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I like the fact that we are searching for a listview in a dialog. If this is an optimization for a custom select menu, then perhaps a constructor config option is in order, for example $("#foo").dialog({ addCornerBottom: false }).

Also, I get what you are caching in the footer var added, but for someone new reading that code, I'm not so sure it's obvious because the collection context changes quite a bit in that chain. ( dialog > header > dialog > content + footer). If we need to cache things, I'm wondering if it will be easier to read if we broke the chain into 2 parts, followed by the config check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To follow on to what Kin is saying, this sort of "lite" coupling between widgets is something we suffer from everywhere in the library. If we want this to be a general purpose configuration for performance reasons (see kin's example) it's fine to put it in the dialog but otherwise if it's widget specific (in this case listview specific) we should find a way to move this to the listview itself.


// this must be an anonymous function so that select menu dialogs can replace
// the close method. This is a change from previously just defining data-rel=back
Expand Down
7 changes: 5 additions & 2 deletions js/jquery.mobile.forms.checkboxradio.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ $.widget( "mobile.checkboxradio", $.mobile.widget, {
});

// Wrap the input + label in a div
input.add( label )
.wrapAll( "<div class='ui-" + inputtype + "'></div>" );
var wrapper = document.createElement('div');
wrapper.setAttribute('class','ui-'+inputtype);
input[0].parentNode.insertBefore(wrapper,input[0]);
wrapper.appendChild(input[0]);
wrapper.appendChild(label[0]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we get confirmation from @toddparker, @scottjehl, or @Wilto as to whether the order of the input and label matters? The original code would preserve the content order in the file, while the new change forces labels to always come after the input.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope—no issues with input/label source order, as far as I know.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that in original version checkboxradio plugin also always places the label after the input element even if the label was set before the input in the markup. I also don’t see any ‘data’ attributes that allow to change this order.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgrebnov

If you look at the implementation of $.fn.add() one of the last things it does (conditionally) is call $.unique() ... this function runs through the resulting merged collection tossing out any duplicates and then performs a sort to reorder the elements in the collection according to the order they appear in the document.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... just to finish off my last comment ... so this:

input.add( label )

will actually produce a collection that reflects the order in which the input and label appear in the document, even though the label was added to the collection containing the input. When wrapAll() is called, the items in the collection are added in the order they appear in the collection, into the new div, which means the order they appeared in the document is preserved.

Knowing about this default document order sorting of elements in the collection is why I originally flagged this change.


label.bind({
vmouseover: function( event ) {
Expand Down
108 changes: 65 additions & 43 deletions js/jquery.mobile.forms.select.custom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"<div class='ui-title'>" + label.getEncodedText() + "</div>"+
"</div>"+
"<div data-" + $.mobile.ns + "role='content'></div>"+
"</div>" ).appendTo( $.mobile.pageContainer ).page(),
"</div>" ),

listbox = $("<div>", { "class": "ui-selectmenu ui-selectmenu-hidden ui-overlay-shadow ui-corner-all ui-body-" + widget.options.overlayTheme + " " + $.mobile.defaultDialogTransition } ).insertAfter(screen),

Expand All @@ -44,9 +44,9 @@
"class": "ui-btn-left"
}).attr( "data-" + $.mobile.ns + "iconpos", "notext" ).attr( "data-" + $.mobile.ns + "icon", "delete" ).appendTo( header ).buttonMarkup(),

menuPageContent = menuPage.find( ".ui-content" ),

menuPageClose = menuPage.find( ".ui-header a" );
menuPageContent,
menuPageClose;


$.extend( widget, {
Expand Down Expand Up @@ -316,6 +316,11 @@
}

if ( menuHeight > screenHeight - 80 || !$.support.scrollTop ) {

self.menuPage.appendTo( $.mobile.pageContainer ).page();
self.menuPageContent = menuPage.find( ".ui-content" );
self.menuPageClose = menuPage.find( ".ui-header a" );

// prevent the parent page from being removed from the DOM,
// otherwise the results of selecting a list item in the dialog
// fall into a black hole
Expand Down Expand Up @@ -344,6 +349,7 @@
self.menuType = "page";
self.menuPageContent.append( self.list );
self.menuPage.find("div .ui-title").text(self.label.text());
self.menuPage.find('.ui-corner-bottom').removeClass('ui-corner-bottom');
$.mobile.changePage( self.menuPage, {
transition: $.mobile.defaultDialogTransition
});
Expand Down Expand Up @@ -408,49 +414,65 @@

self.list.empty().filter( ".ui-listview" ).listview( "destroy" );

// Populate menu with options from select element
self.select.find( "option" ).each( function( i ) {
var $this = $( this ),
$parent = $this.parent(),
text = $this.getEncodedText(),
anchor = "<a href='#'>"+ text +"</a>",
classes = [],
extraAttrs = [];

// Are we inside an optgroup?
if ( $parent.is( "optgroup" ) ) {
var optLabel = $parent.attr( "label" );

// has this optgroup already been built yet?
if ( $.inArray( optLabel, optgroups ) === -1 ) {
lis.push( "<li data-" + $.mobile.ns + "role='list-divider'>"+ optLabel +"</li>" );
optgroups.push( optLabel );
var $options = self.select.find("option"),
numOptions = $options.length,
select = this.select[ 0 ],
dataPrefix = 'data-' + $.mobile.ns,
dataIndexAttr = dataPrefix + 'option-index',
dataIconAttr = dataPrefix + 'icon',
dataRoleAttr = dataPrefix + 'role',
fragment = document.createDocumentFragment(),
optGroup;

for (var i = 0; i < numOptions;i++){
var option = $options[i],
$option = $(option),
parent = option.parentNode,
text = $option.text(),
anchor = document.createElement('a');
classes = [];

anchor.setAttribute('href','#');
anchor.appendChild(document.createTextNode(text));

// Are we inside an optgroup?
if (parent !== select && parent.nodeName.toLowerCase() === "optgroup"){
var optLabel = parent.getAttribute('label');
if ( optLabel != optGroup) {
var divider = document.createElement('li');
divider.setAttribute(dataRoleAttr,'list-divider');
divider.setAttribute('role','option');
divider.setAttribute('tabindex','-1');
divider.appendChild(document.createTextNode(optLabel));
fragment.appendChild(divider);
optGroup = optLabel;
}
}

if (!placeholder){
if ( !option.getAttribute( "value" ) || text.length == 0 || $option.jqmData( "placeholder" ) ) {
if ( o.hidePlaceholderMenuItems ) {
classes.push( "ui-selectmenu-placeholder" );
}
placeholder = self.placeholder = text;
}
}

// Find placeholder text
// TODO: Are you sure you want to use getAttribute? ^RW
if ( !this.getAttribute( "value" ) || text.length == 0 || $this.jqmData( "placeholder" ) ) {
if ( o.hidePlaceholderMenuItems ) {
classes.push( "ui-selectmenu-placeholder" );
}
placeholder = self.placeholder = text;
}

// support disabled option tags
if ( this.disabled ) {

var item = document.createElement('li');
if ( option.disabled ) {
classes.push( "ui-disabled" );
extraAttrs.push( "aria-disabled='true'" );
item.setAttribute('aria-disabled',true);
}

lis.push( "<li data-" + $.mobile.ns + "option-index='" + i + "' data-" + $.mobile.ns + "icon='"+ dataIcon +"' class='"+ classes.join(" ") + "' " + extraAttrs.join(" ") +">"+ anchor +"</li>" );
});

self.list.html( lis.join(" ") );

self.list.find( "li" )
.attr({ "role": "option", "tabindex": "-1" })
.first().attr( "tabindex", "0" );
item.setAttribute(dataIndexAttr,i);
item.setAttribute(dataIconAttr,dataIcon);
item.setAttribute('class',classes.join(" "));
item.setAttribute('role','option');
item.setAttribute('tabindex','-1');
item.appendChild(anchor);
fragment.appendChild(item);
}
fragment.firstChild.setAttribute('tabindex',0);
self.list[0].appendChild(fragment);

// Hide header close link for single selects
if ( !this.isMultiple ) {
Expand Down
69 changes: 40 additions & 29 deletions js/jquery.mobile.forms.slider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,27 @@ $.widget( "mobile.slider", $.mobile.widget, {

step = window.parseFloat( control.attr( "step" ) || 1 ),

slider = $( "<div class='ui-slider " + selectClass + " ui-btn-down-" + trackTheme +
" ui-btn-corner-all' role='application'></div>" ),

handle = $( "<a href='#' class='ui-slider-handle'></a>" )
.appendTo( slider )
.buttonMarkup({ corners: true, theme: theme, shadow: true })
.attr({
"role": "slider",
"aria-valuemin": min,
"aria-valuemax": max,
"aria-valuenow": val(),
"aria-valuetext": val(),
"title": val(),
"aria-labelledby": labelID
}),
domSlider = document.createElement('div'),

handle = document.createElement('a');

domSlider.setAttribute('role','application');
domSlider.setAttribute('class',['ui-slider ',selectClass," ui-btn-down-",trackTheme,' ui-btn-corner-all'].join(""));
handle.setAttribute('class','ui-slider-handle');
domSlider.appendChild(handle);

var slider = $(domSlider),
handle = $(handle)
.buttonMarkup({ corners: true, theme: theme, shadow: true })
.attr({
"role": "slider",
"aria-valuemin": min,
"aria-valuemax": max,
"aria-valuenow": val(),
"aria-valuetext": val(),
"title": val(),
"aria-labelledby": labelID
}),
options;

$.extend( this, {
Expand All @@ -72,27 +78,32 @@ $.widget( "mobile.slider", $.mobile.widget, {
});

if ( cType == "select" ) {

slider.wrapInner( "<div class='ui-slider-inneroffset'></div>" );

var wrapper = document.createElement('div');
wrapper.setAttribute('class','ui-slider-inneroffset');

for(var j = 0,length = domSlider.childNodes.length;j < length;j++){
wrapper.appendChild(domSlider.childNodes[j]);
}
domSlider.appendChild(wrapper);

// make the handle move with a smooth transition
handle.addClass( "ui-slider-handle-snapping" );

options = control.find( "option" );

control.find( "option" ).each(function( i ) {

for(var i = 0, length = options.length; i < length; i++){
var side = !i ? "b":"a",
corners = !i ? "right" :"left",
theme = !i ? " ui-btn-down-" + trackTheme :( " " + $.mobile.activeBtnClass );

$( "<div class='ui-slider-labelbg ui-slider-labelbg-" + side + theme + " ui-btn-corner-" + corners + "'></div>" )
.prependTo( slider );

$( "<span class='ui-slider-label ui-slider-label-" + side + theme + " ui-btn-corner-" + corners + "' role='img'>" + $( this ).getEncodedText() + "</span>" )
.prependTo( handle );
});

theme = !i ? " ui-btn-down-" + trackTheme :( " " + $.mobile.activeBtnClass ),
sliderLabel = document.createElement('div'),
sliderImg = document.createElement('span');
sliderLabel.setAttribute('class',['ui-slider-labelbg ui-slider-labelbg-',side,theme," ui-btn-corner-",corners].join(""));
$(sliderLabel).prependTo( slider );
sliderImg.setAttribute('class',['ui-slider-label ui-slider-label-',side,theme," ui-btn-corner-",corners].join(""));
sliderImg.setAttribute('role','img');
sliderImg.appendChild(document.createTextNode(options[i].innerHTML));
$(sliderImg).prependTo( handle );
}
}

label.addClass( "ui-slider" );
Expand Down