Fix recent pitfall in toggle component.

This is a recent regression where we I refactored the toggle
component.  For some reason the old code was waiting until
after the callback to set some of its state, and I did the
same thing when I simplified how the state was stored.

Under the old code, this didn't manifest as a bug, although
the old code was problematic for other reasons.

This "fix" doesn't actually change anything user facing, as the
follow up commit fixes the proximal problem more directly. And
the toggle component is still prone to people writing code that
tries to inspect the state of the widget as it's being built.
This commit is contained in:
Steve Howell 2018-04-04 15:06:58 -04:00 committed by Tim Abbott
parent 0e7073ec29
commit 27770d7f6b
2 changed files with 12 additions and 3 deletions

View File

@ -121,7 +121,10 @@ var RIGHT_KEY = { which: 39, preventDefault: noop };
} }
}); });
var widget = components.toggle({ var callback_value;
var widget;
widget = components.toggle({
name: "info-overlay-toggle", name: "info-overlay-toggle",
selected: 0, selected: 0,
values: [ values: [
@ -132,6 +135,12 @@ var RIGHT_KEY = { which: 39, preventDefault: noop };
callback: function (name, key) { callback: function (name, key) {
assert.equal(callback_args, undefined); assert.equal(callback_args, undefined);
callback_args = [name, key]; callback_args = [name, key];
// The subs code tries to get a widget value in the middle of a
// callback, which can lead to obscure bugs.
if (widget) {
callback_value = widget.value();
}
}, },
}); });
@ -165,6 +174,7 @@ var RIGHT_KEY = { which: 39, preventDefault: noop };
assert.equal(tabs[2].class, 'last selected'); assert.equal(tabs[2].class, 'last selected');
assert.deepEqual(callback_args, ['translated: Search operators', 'search-operators']); assert.deepEqual(callback_args, ['translated: Search operators', 'search-operators']);
assert.equal(widget.value(), 'translated: Search operators'); assert.equal(widget.value(), 'translated: Search operators');
assert.equal(widget.value(), callback_value);
// try to crash the key handler // try to crash the key handler
keydown_f.call(tabs[focused_tab], RIGHT_KEY); keydown_f.call(tabs[focused_tab], RIGHT_KEY);

View File

@ -52,6 +52,7 @@ exports.toggle = (function () {
elem.addClass("selected"); elem.addClass("selected");
if (idx !== meta.idx) { if (idx !== meta.idx) {
meta.idx = idx;
if (opts.callback) { if (opts.callback) {
opts.callback( opts.callback(
opts.values[idx].label, opts.values[idx].label,
@ -61,8 +62,6 @@ exports.toggle = (function () {
} }
} }
meta.idx = idx;
if (!opts.child_wants_focus) { if (!opts.child_wants_focus) {
elem.focus(); elem.focus();
} }