Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Issue #385: urlConfig select-one lists #443

Closed
wants to merge 4 commits into from

3 participants

@gibson042

See image at #385 for UI

@jzaefferer
Owner

There's a jshint issue (see Travis results). Could you also provide a PR against the API docs? https://github.com/jquery/api.qunitjs.com/blob/master/entries/QUnit.config.xml

That PR would probably answer my questions about the usage of this feature.

Apart from that I'm concerned about all the html building string concatenation. If we land this PR as-is, we need to refactor that part later.

@jzaefferer
Owner

@gibson042 can you provide an example of how this should be used?

@jzaefferer
Owner

@dmethvin do you know anything about this? Or how to reach @gibson042 besides GitHub notifications?

@jzaefferer
Owner

Nevermind, finally noticed the ticket reference. No idea how I missed that before.

@gibson042

@jzaefferer Sorry for dropping this... it's an implementation of @Krinkle's suggestions at #385 (comment). I agree on the string concatenation, but chose to fit the existing style rather than muddy the waters with a sweeping refactor at this time. Is there anything else you still need from me here?

@jzaefferer
Owner

@gibson042 sorry for not touching this for so long. Would you be willing to redo this on top of current master?

@gibson042

@jzaefferer Done... it merged clean after mapping changes from qunit/qunit.js to src/core.js.

@jzaefferer
Owner

Thanks @gibson042. I've tried testing this, but couldn't figure out how to actually configure this. Could you extend one of the existing testsuites to include a urlConfig that makes use of this feature?

Would also be nice to document this on the API site

@gibson042

@jzaefferer I couldn't see anywhere in the tests to actually verify something like urlConfig, and wasn't feeling quite ambitious enough to create that today. But I did put up a small demo and can extend it as needed.

I also opened a sister PR for api.qunitjs.com, though I'm pretty sure it needs work: jquery/api.qunitjs.com#42

@Krinkle Krinkle commented on the diff
src/core.js
((7 lines not shown))
};
}
config[ val.id ] = QUnit.urlParams[ val.id ];
- urlConfigHtml += "<input id='qunit-urlconfig-" + escapeText( val.id ) +
- "' name='" + escapeText( val.id ) +
- "' type='checkbox'" + ( config[ val.id ] ? " checked='checked'" : "" ) +
- " title='" + escapeText( val.tooltip ) +
- "'><label for='qunit-urlconfig-" + escapeText( val.id ) +
- "' title='" + escapeText( val.tooltip ) + "'>" + val.label + "</label>";
+ if ( !val.value || typeof val.value === "string" ) {
+ urlConfigHtml += "<input id='qunit-urlconfig-" + escapeText( val.id ) +
+ "' name='" + escapeText( val.id ) +
+ "' type='checkbox'" +
+ ( val.value ? " value='" + escapeText( val.value ) + "'" : "" ) +
@Krinkle Collaborator
Krinkle added a note

The default value was previously (implicitly) set to "on", not sure whether an empty string won't break anything. I imagine it might break the boolean cast assumption (the old settings didn't support custom values for checkboxes, only true/false which was implemented as undefined (?) and "on" (?{id}=on).

In the case of undefined value, this preserves the old behavior of omitting the corresponding attribute from resulting checkboxes, which is assumed "true" by the event handlers: gibson042@c7c466f^...c621f60#diff-7eb52b366866677666470e019283c8eaL665

@Krinkle Collaborator
Krinkle added a note

Sorry, I misread the code. I thought it was setting value="" but it isn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/core.js
((19 lines not shown))
+ "' type='checkbox'" +
+ ( val.value ? " value='" + escapeText( val.value ) + "'" : "" ) +
+ ( config[ val.id ] ? " checked='checked'" : "" ) +
+ " title='" + escapeText( val.tooltip ) +
+ "'><label for='qunit-urlconfig-" + escapeText( val.id ) +
+ "' title='" + escapeText( val.tooltip ) + "'>" + val.label + "</label>";
+ } else {
+ urlConfigHtml += "<label for='qunit-urlconfig-" + escapeText( val.id ) +
+ "' title='" + escapeText( val.tooltip ) +
+ "'>" + val.label +
+ ": </label><select id='qunit-urlconfig-" + escapeText( val.id ) +
+ "' name='" + escapeText( val.id ) +
+ "' title='" + escapeText( val.tooltip ) +
+ "'><option></option>";
+ selection = false;
+ if ( toString.call( val.value ) === "[object Array]" ) {
@Krinkle Collaborator
Krinkle added a note

Use the is( obj, type ) utility function instead.

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jzaefferer jzaefferer closed this pull request from a commit
@gibson042 gibson042 Core: Extend QUnit.config.urlConfig to support select-one dropdowns
To make it easier for projects like jQuery UI to provide a dropdown that
let's the user select the jQuery Core version to test against. Can
display any key/value pairs (or just keys). Selection will be available
via `location.search` and parsed as `QUnit.config`.

Fixes gh-385
Closes gh-443
1af7edb
@jzaefferer
Owner

Landed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 3, 2014
  1. @gibson042
  2. @gibson042
  3. @gibson042

    fix jshint error

    gibson042 authored
Commits on Jan 28, 2014
  1. @gibson042
This page is out of date. Refresh to see the latest.
Showing with 62 additions and 17 deletions.
  1. +62 −17 src/core.js
View
79 src/core.js
@@ -545,8 +545,8 @@ QUnit.load = function() {
runLoggingCallbacks( "begin", QUnit, {} );
// Initialize the config, saving the execution queue
- var banner, filter, i, label, len, main, ol, toolbar, userAgent, val,
- urlConfigCheckboxesContainer, urlConfigCheckboxes, moduleFilter,
+ var banner, filter, i, j, label, len, main, ol, toolbar, val, selection,
+ urlConfigContainer, moduleFilter, userAgent,
numModules = 0,
moduleNames = [],
moduleFilterHtml = "",
@@ -565,17 +565,55 @@ QUnit.load = function() {
if ( typeof val === "string" ) {
val = {
id: val,
- label: val,
- tooltip: "[no tooltip available]"
+ label: val
};
}
config[ val.id ] = QUnit.urlParams[ val.id ];
- urlConfigHtml += "<input id='qunit-urlconfig-" + escapeText( val.id ) +
- "' name='" + escapeText( val.id ) +
- "' type='checkbox'" + ( config[ val.id ] ? " checked='checked'" : "" ) +
- " title='" + escapeText( val.tooltip ) +
- "'><label for='qunit-urlconfig-" + escapeText( val.id ) +
- "' title='" + escapeText( val.tooltip ) + "'>" + val.label + "</label>";
+ if ( !val.value || typeof val.value === "string" ) {
+ urlConfigHtml += "<input id='qunit-urlconfig-" + escapeText( val.id ) +
+ "' name='" + escapeText( val.id ) +
+ "' type='checkbox'" +
+ ( val.value ? " value='" + escapeText( val.value ) + "'" : "" ) +
@Krinkle Collaborator
Krinkle added a note

The default value was previously (implicitly) set to "on", not sure whether an empty string won't break anything. I imagine it might break the boolean cast assumption (the old settings didn't support custom values for checkboxes, only true/false which was implemented as undefined (?) and "on" (?{id}=on).

In the case of undefined value, this preserves the old behavior of omitting the corresponding attribute from resulting checkboxes, which is assumed "true" by the event handlers: gibson042@c7c466f^...c621f60#diff-7eb52b366866677666470e019283c8eaL665

@Krinkle Collaborator
Krinkle added a note

Sorry, I misread the code. I thought it was setting value="" but it isn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ ( config[ val.id ] ? " checked='checked'" : "" ) +
+ " title='" + escapeText( val.tooltip ) +
+ "'><label for='qunit-urlconfig-" + escapeText( val.id ) +
+ "' title='" + escapeText( val.tooltip ) + "'>" + val.label + "</label>";
+ } else {
+ urlConfigHtml += "<label for='qunit-urlconfig-" + escapeText( val.id ) +
+ "' title='" + escapeText( val.tooltip ) +
+ "'>" + val.label +
+ ": </label><select id='qunit-urlconfig-" + escapeText( val.id ) +
+ "' name='" + escapeText( val.id ) +
+ "' title='" + escapeText( val.tooltip ) +
+ "'><option></option>";
+ selection = false;
+ if ( QUnit.is( "array", val.value ) ) {
+ for ( j = 0; j < val.value.length; j++ ) {
+ urlConfigHtml += "<option value='" + escapeText( val.value[j] ) + "'" +
+ ( config[ val.id ] === val.value[j] ?
+ (selection = true) && " selected='selected'" :
+ "" ) +
+ ">" + escapeText( val.value[j] ) + "</option>";
+ }
+ } else {
+ for ( j in val.value ) {
+ if ( hasOwn.call( val.value, j ) ) {
+ urlConfigHtml += "<option value='" + escapeText( j ) + "'" +
+ ( config[ val.id ] === j ?
+ (selection = true) && " selected='selected'" :
+ "" ) +
+ ">" + escapeText( val.value[j] ) + "</option>";
+ }
+ }
+ }
+ if ( config[ val.id ] && !selection ) {
+ urlConfigHtml += "<option value='" + escapeText( config[ val.id ] ) +
+ "' selected='selected' disabled='disabled'>" +
+ escapeText( config[ val.id ] ) +
+ "</option>";
+ }
+ urlConfigHtml += "</select>";
+ }
}
for ( i in config.modules ) {
if ( config.modules.hasOwnProperty( i ) ) {
@@ -652,20 +690,27 @@ QUnit.load = function() {
label.innerHTML = "Hide passed tests";
toolbar.appendChild( label );
- urlConfigCheckboxesContainer = document.createElement("span");
- urlConfigCheckboxesContainer.innerHTML = urlConfigHtml;
- urlConfigCheckboxes = urlConfigCheckboxesContainer.getElementsByTagName("input");
+ urlConfigContainer = document.createElement("span");
+ urlConfigContainer.innerHTML = urlConfigHtml;
// For oldIE support:
// * Add handlers to the individual elements instead of the container
- // * Use "click" instead of "change"
+ // * Use "click" instead of "change" for checkboxes
// * Fallback from event.target to event.srcElement
- addEvents( urlConfigCheckboxes, "click", function( event ) {
+ addEvents( urlConfigContainer.getElementsByTagName("input"), "click", function( event ) {
+ var params = {},
+ target = event.target || event.srcElement;
+ params[ target.name ] = target.checked ?
+ target.defaultValue || true :
+ undefined;
+ window.location = QUnit.url( params );
+ });
+ addEvents( urlConfigContainer.getElementsByTagName("select"), "change", function( event ) {
var params = {},
target = event.target || event.srcElement;
- params[ target.name ] = target.checked ? true : undefined;
+ params[ target.name ] = target.options[ target.selectedIndex ].value || undefined;
window.location = QUnit.url( params );
});
- toolbar.appendChild( urlConfigCheckboxesContainer );
+ toolbar.appendChild( urlConfigContainer );
if (numModules > 1) {
moduleFilter = document.createElement( "span" );
Something went wrong with that request. Please try again.