As discussed at length in patch [[ https://gerrit.wikimedia.org/r/c/mediawiki/core/+/606306/29 | 606306 - checkboxHack: Trigger 'input' event when checkbox state changes ]] the public API for the checkBox module could be improved by converting it from a model where we have multiple initialization functions to one where we have a single initialization function with an options object and a callback function to hook into the checkbox's change events.
From @Krinkle's comments:---
As summarized by @Demian in T254851#6270677 :
```
Right now it seems more than half the code here and there is just exposing lots and lots of options,
and then setting up all those optionsIn [[ https://gerrit.wikimedia.org/r/c/mediawiki/core/+/606306/24#message-f29ccc0d63346b9497a42eb3aff6a829a60db07b | discussion ]] with @Krinkle it came up that the interface of CheckboxHack is not simple enough and the responsibilities of the module and its users aren't clearly separated. I suppose one way this could work for Vector iThe interface now looks like:
var checkboxHack = require(…);
checkboxHack.create(
checkbox,
function ( /* bool */ changedState ) {
…callback on change…```lang=js
}
);
That's all :)
I'm not sure why having "click", "space" and "enter" work is optional, or why aria binding is optional
or why even with aria binding on, why setting aria initially is also optional. Although if we do have
multiple differing use cases, I suppose create() could have a options object as second parameter.module.exports = {
```
The new API, as laid out in @Demian's [[ https://gerrit.wikimedia.org/r/c/mediawiki/core/+/608744 | patches ]], could look like the following:
```lang=js updateAriaExpanded: updateAriaExpanded,
/** bindUpdateAriaExpandedOnInput: bindUpdateAriaExpandedOnInput,
* Checkbox hack bindToggleOnClick: binding type.ToggleOnClick,
* bindDismissOnClickOutside: bindDismissOnClickOutside,
* @class {Object} CheckboxHack bindDismissOnFocusLoss: bindDismissOnFocusLoss,
* @property {Window} window bind: bind,
* @property {HTMLInputElement} checkbox unbind: unbind
* @property {HTMLElement} button };
* @property {CheckboxHackOptions} op```
It's the client's responsibility to properly combine these functions.
* @property {Function} unbindI've reduced this into:
* @ignore```lang=js
*/
/**
* Checkbox hack initialization options type.
** Public API
* @class {Object} CheckboxHackOptions*/
* @property {boolean} [noClickHandler]module.exports = CheckboxHack;
* @property {boolean} [noKeyHandler]```
* @property {Node} [autoHideElement]Usage:
* @property {Function} [onChange]```lang=js
checkboxHack = new CheckboxHack( window, * @ignore
checkbox, button, */{ onChange: saveSidebarState } );
```
in practice:
```lang=js
var checkboxHack = new CheckboxHack(
window,
checkbox,
button,
{
noClickHandler: false, // default - disables JS handling of click/touch on the checkbox.
noKeyHandler: false, // default - disables space/enter key handling.
autoHideElement: menuElement // formally `bindDismissOnClickOutside` a target element to hide when clicking outside of itFurther named parameters can be provided in the options object to disable individual features, such as click and key handling.
==== Benefit
No need to change Vector when the CheckboxHack's implementation is changed, eg. [[https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/608451|608451]].
onChange: function(checkboxHackClean, progressively extensible API, separation of concerns.
==== Patch dependency graph
[ [[https://gerrit.wikimedia.org/r/c/mediawiki/core/+/608744|1st core patch: 608744]] ] -> [ [[https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/606541|Vector patch: 606541]]] -> [ [[https://gerrit.wikimedia.org/r/c/mediawiki/core/+/608755|5th in core: 608755]] "Finish removing internal functions from public API"] -> [ [[https://gerrit.wikimedia.org/r/c/mediawiki/core/+/608756|608756]] "onChange[] callback"]
This sequence enables SPACE, ENTER and a hook for saving the sidebar state for T255727.
The patches are minimal, focused on just one change to make review easier. If wanted, event) {} // callback function to execute when checkbox state changesthe patches can be squashed after being reviewed.
} );The code is augmented with strict type annotations, prepared for all jsduck, eslint and tsc. Briefly: type safe. The difficulty was TS and jsduck understanding a bit different dialect.
```
After this refactor is complete, we will have to update Vector to utilize the new API, and then remove the old API.
The implementation of this new API has already been undertaken by @Demian starting with this patch: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/608744