New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow text input on slider widgets #6106
Changes from 6 commits
66b32ee
c6ecc98
d741bc7
cdab764
71d59e8
e4c2069
8d226ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ define([ | |
this.$readout = $('<div/>') | ||
.appendTo(this.$el) | ||
.addClass('widget-readout') | ||
.attr('contentEditable', true) | ||
.hide(); | ||
|
||
this.model.on('change:slider_color', function(sender, value) { | ||
|
@@ -156,9 +157,84 @@ define([ | |
|
||
events: { | ||
// Dictionary of events and their handlers. | ||
"slide" : "handleSliderChange" | ||
"slide" : "handleSliderChange", | ||
"blur [contentEditable=true]": "handleTextChange", | ||
"keydown [contentEditable=true]": "handleKeyDown" | ||
}, | ||
|
||
handleKeyDown: function(e) { | ||
if (e.keyCode == 13) { | ||
e.preventDefault(); | ||
this.handleTextChange(); | ||
} | ||
}, | ||
|
||
handleTextChange: function() { | ||
// this handles the entry of text into the contentEditable label | ||
// first, the value is checked if it contains a parseable number | ||
// (or pair of numbers, for the _range case) | ||
// then it is clamped within the min-max range of the slider | ||
// finally, the model is updated if the value is to be changed | ||
// | ||
// if any of these conditions are not met, the text is reset | ||
// | ||
// the step size is not enforced | ||
|
||
var text = this.$readout.text(); | ||
var vmin = this.model.get('min'); | ||
var vmax = this.model.get('max'); | ||
if (this.model.get("_range")) { | ||
// range case | ||
// ranges can be expressed either "val-val" or "val:val" (+spaces) | ||
var match = this._range_regex.exec(text); | ||
if (match) { | ||
var values = [this._parse_value(match[1]), | ||
this._parse_value(match[2])]; | ||
// reject input where NaN or lower > upper | ||
if (isNaN(values[0]) || | ||
isNaN(values[1]) || | ||
(values[0] > values[1])) { | ||
this.$readout.text(this.model.get('value').join('-')); | ||
} else { | ||
// clamp to range | ||
values = [Math.max(Math.min(values[0], vmax), vmin), | ||
Math.max(Math.min(values[1], vmax), vmin)]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At first glance I thought this would cause trouble, since you do not check if min and max are undefined, but the widget won't exist in the front-end until it's been opened from the back-end, and the second it's opened from the back-end, a full state is pushed. When a widget like this is opened from the front-end, it will be important to remember that some values have to be defined on the model before it will work properly (this is probably not the only place) @jasongrout @SylvainCorlay . |
||
|
||
if ((values[0] != this.model.get('value')[0]) || | ||
(values[1] != this.model.get('value')[1])) { | ||
this.$readout.text(values.join('-')); | ||
this.model.set('value', values, {updated_view: this}); | ||
this.touch(); | ||
} else { | ||
this.$readout.text(this.model.get('value').join('-')); | ||
} | ||
} | ||
} else { | ||
this.$readout.text(this.model.get('value').join('-')); | ||
} | ||
} else { | ||
// single value case | ||
var value = this._parse_value(text); | ||
if (isNaN(value)) { | ||
this.$readout.text(this.model.get('value')); | ||
} else { | ||
value = Math.max(Math.min(value, vmax), vmin); | ||
|
||
if (value != this.model.get('value')) { | ||
this.$readout.text(value); | ||
this.model.set('value', value, {updated_view: this}); | ||
this.touch(); | ||
} else { | ||
this.$readout.text(this.model.get('value')); | ||
} | ||
} | ||
} | ||
}, | ||
|
||
_parse_value: parseInt, | ||
|
||
_range_regex: /^\s*([+-]?\d+)\s*[-:]\s*([+-]?\d+)/, | ||
|
||
handleSliderChange: function(e, ui) { | ||
// Called when the slider value is changed. | ||
|
||
|
@@ -286,10 +362,7 @@ define([ | |
} | ||
}, | ||
|
||
_parse_value: function(value) { | ||
// Parse the value stored in a string. | ||
return parseInt(value); | ||
}, | ||
_parse_value: parseInt | ||
}); | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to require
static/base/js/keyboard
and userkeycode.enter
here ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for this suggestion