Improve Customizer JSDoc - #10743
Improve Customizer JSDoc#10743westonruter wants to merge 48 commits into
Conversation
Co-authored-by: shailu25 <shailu25@git.wordpress.org> Co-authored-by: vishalkakadiya <vishalkakadiya@git.wordpress.org>
Per Gemini:
I've completed the JSDoc improvements in the `src/js/_enqueues/wp/customize/` directory.
Summary of changes:
- Corrected missing braces around types in `@param` and `@return` tags across several files.
- Replaced non-standard return types like `{wp.customize.controlConstructor.menus[]}` with more accurate instance types like `{wp.customize.Control}` or `{wp.customize.Control[]}`.
- Fixed placeholder JSDoc like `[type]` and `[description]` in `base.js`.
- Updated descriptions to use "jQuery object" instead of "jQuery collection" for consistency.
- Improved formatting for nested parameters in `Messenger.initialize`.
- Corrected a parameter name mismatch in `api.Class.extend`.
All changes have been verified with `svn diff`.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Gemini: I have completed the requested JSDoc improvements for the Customizer JavaScript files based on the requirements of ticket #40831. All local changes are confined to JSDoc blocks and have been verified. I am now finished with the task. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request improves JSDoc documentation across multiple WordPress Customizer JavaScript files. The changes add missing documentation blocks, clarify parameter types, add return type annotations, and update imprecise type references to more accurate generic types.
Changes:
- Added comprehensive JSDoc comments for previously undocumented methods in views, models, and loader files
- Updated return type annotations from specific control constructor types to generic
wp.customize.Controltypes for better accuracy - Added missing
@return {void}tags for event handler methods and@sincetags where appropriate
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/js/_enqueues/wp/customize/widgets.js | Updated return types for control-related methods from specific constructor types to generic Control types |
| src/js/_enqueues/wp/customize/views.js | Added comprehensive JSDoc for HeaderTool view methods including initialize, render, and helper functions |
| src/js/_enqueues/wp/customize/preview.js | Added parameter documentation for debounce function and return type tags for event handlers |
| src/js/_enqueues/wp/customize/preview-nav-menus.js | Added @SInCE tags, parameter documentation, and return type annotations for nav menu preview functions |
| src/js/_enqueues/wp/customize/nav-menus.js | Updated return types from specific control constructor types to generic Control types and improved parameter documentation |
| src/js/_enqueues/wp/customize/models.js | Added comprehensive JSDoc for HeaderTool model methods including initialize, comparator, and utility functions |
| src/js/_enqueues/wp/customize/loader.js | Added JSDoc for event handler methods and improved parameter documentation for state management functions |
| src/js/_enqueues/wp/customize/base.js | Improved parameter and return type documentation for core utility functions and classes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Fold in the JSDoc corrections made to `src/js/_enqueues/wp/customize/` in WordPress#13251 so that those files can be dropped from that pull request. Where that pull request removed a `@param` tag because the documented variadic had no corresponding named parameter for `jsdoc/check-param-names` to match, adopt rest syntax instead of dropping the documentation. This preserves — and in several cases restores — the description of what the extra arguments mean: * `wp.customize.Value#bind()`, `#unbind()`, `#link()`, `#unlink()`, `#sync()` and `#unsync()`. The latter four previously carried only a trailing `// values*` comment, now replaced by real `@param` tags. * `wp.customize.Values#instance()`, `#create()` and `#when()`. * `wp.customize.Events#trigger()`, `#bind()` and `#unbind()`, which gain docblocks they never had. Convert the remaining uses of `arguments` in these files to rest parameters, or to a direct `call()` where the receiving method declares a fixed signature. Two of these are worth noting: * `wp.customize.Widgets.WidgetControl` forwards to a `widget-synced` handler that takes a third `newForm` argument the listener does not declare, so it keeps forwarding via rest rather than collapsing to a fixed `call()`. * `wp.customize.Class` keeps `arguments`, since it is passed on to `initialize()` and must reflect the number of arguments actually supplied. Replace `wp.customize.controlConstructor.*` return types, which name a constructor where an instance is meant, with the documented control classes: `wp.customize.Menus.MenuControl`, `wp.customize.Menus.MenuItemControl`, `wp.customize.Widgets.SidebarControl` and `wp.customize.Widgets.WidgetControl`. `Array.prototype.slice` is no longer referenced in customize-base.js and is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`customize-base.js` wraps its contents in an IIFE whose first parameter is
named `exports`, because that is precisely what it is used for:
(function( exports, $ ){
…
exports.customize = api;
})( wp, jQuery );
That header was copied to `customize-controls.js`, `customize-preview.js` and
`customize-loader.js`, but in those three files the parameter is never
referenced. Each one reaches for the global `wp` instead, so the argument
being passed in is silently discarded.
Rename the parameter to `wp` in those three files so that the argument is
actually consumed and the global lookups resolve to a local binding. This
matches `customize-widgets.js`, which already declares `(function( wp, $ ){`.
`customize-base.js` is left alone, as `exports` is both used there and
descriptive of its role.
Co-Authored-By: Andrea Fercia <afercia@git.wordpress.org>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`wp.customize.Class` accepts arguments in two forms. Normally they are passed straight through to the class's `initialize` method, which is how nearly every instance in the Customizer is constructed. As a special case, when the first argument is `wp.customize.Class.applicator`, the second argument is the array of arguments for `initialize` and the third extends the instance. Collecting the direct form requires the number of arguments actually supplied, which is why `arguments` was used here. Declaring the three named parameters and collecting only the remainder with a rest parameter would not preserve that: rebuilding the list as `[ applicator, argsArray, options, ...rest ]` always yields at least three entries, so `new wp.customize.Value( true )` would call `initialize()` with three arguments rather than one. Collecting every argument with a single rest parameter and indexing into it instead is exactly equivalent, since the resulting array has the original length. Document both forms while here. The previous docblock described only the applicator form, which has a single call site, and not the direct form used everywhere else. Also rename the rest parameter of the `instance` wrapper, which shadowed the outer `args`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The docblocks added for `link()` and `unlink()` described the relationship backwards, saying that the value's changes are propagated to the supplied values. It is the other way around: `link()` binds this value's setter as a callback on each supplied value, so this value follows them. The call sites read that way too. `Messenger` derives `origin` from `url`, an input element follows its setting, and the selected changeset status follows the changeset status so that updates made on the server are reflected in the selection. Note the one-directional nature of this, since `sync()` is the method that links in both directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The existing tests exercised these classes only through their simplest calls, passing a single callback or a single value, so nothing confirmed the behavior of the methods that accept any number of arguments. Add tests for: * `Class.applicator`, resolving a longstanding `@todo`. One test covers the arguments being taken from the supplied array, and another covers the instance being extended before `initialize()` runs. * The number of arguments `Class` passes to `initialize()`, which has to match the number it was given. * An instance being callable as a function when the class defines an `instance()` method, resolving the other `@todo`. * `Value#bind()` and `Value#unbind()` with more than one callback. * `Value#link()` and `Value#unlink()`, including that following a value is one-directional, and `Value#sync()` and `Value#unsync()`. * `Values#create()` passing its extra arguments through to `initialize()`. * `Values#when()` waiting for a value that does not exist yet. The suite wraps every test with sinon's fake timers, so the promise returned by `when()` does not resolve until the clock is advanced. Advance it rather than waiting, which keeps the test synchronous. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`mixed` is a PHP type. JSDoc spells the any type as `*`, and TypeScript, which checks a growing number of the files in `src/js` by way of `tsconfig.json`, reports `Cannot find name 'mixed'` for it. Ten occurrences across customize-base.js, customize-controls.js and customize-views.js are updated, and the surrounding parameter descriptions are realigned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Several types were written as bare class names, or in terms of the `api` alias
that the Customizer files use internally for `wp.customize`. Neither form
resolves, since the documented names are the public ones:
* `{Value}` becomes `{wp.customize.Value}`.
* `{Placement}` and `{Partial}` become
`{wp.customize.selectiveRefresh.Placement}` and
`{wp.customize.selectiveRefresh.Partial}`.
* `{api.Notification}` becomes `{wp.customize.Notification}`, and
`{api.selectiveRefresh.Placement}` becomes its `wp.customize` equivalent.
* `@see {api.Values.when}` becomes `@see {@link wp.customize.Values#when}`,
matching how the other cross references in these files are written.
Two `@lends` annotations in customize-selective-refresh.js named the wrong
symbol, so the members they introduce were attached to something that does not
exist. `Partial` used `wp.customize.SelectiveRefresh`, which is capitalized
differently to the `wp.customize.selectiveRefresh` namespace it belongs to, and
`Placement` lent its members to the namespace itself rather than to
`Placement`.
Also correct two malformed types. `{event}` is not a type; the parameter is a
jQuery event, as it is everywhere else in customize-controls.js. And a default
value belongs on the parameter name rather than inside the braces, so
`{boolean=true} [options.triggerRendered]` becomes `{boolean}
[options.triggerRendered=true]`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `$` argument of these functions is jQuery itself, not a collection of
elements, so `{jQuery}` describes the wrong thing. The type of the `jQuery`
global is `JQueryStatic`, which is the name `@types/jquery` exports and the one
`typings/wp-globals/index.d.ts` already refers to.
The `wp` and `_` arguments were given `{wp}` and `{_}`, which name the globals
being passed rather than any type. These become `{Object}`, matching how
customize-preview-widgets.js already describes the same two arguments.
For the same reason `{window}` becomes `{Window}` in the Messenger docblock,
naming the interface rather than the global.
While here, move the arguments of the customize-views.js function out of the
file's `@output` docblock and into one of its own, attached to the function.
The other files here already keep the two separate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| }, | ||
|
|
||
| /** | ||
| * Maybe add random choice. |
There was a problem hiding this comment.
I love the determinism of this!!! 🤪
Found by running the Customizer files through a stricter set of the rules that `eslint-plugin-jsdoc` offers than `.eslintrc-jsdoc.js` currently enables, notably `no-undefined-types`, `valid-types`, `check-access`, `check-alignment` and `no-bad-blocks`. * The `@callback` definitions for the deferred control, section, panel and notification callbacks are declared under `wp.customize`, but were referred to by their bare names, which do not resolve. * `params.message=null` gave a default for a parameter that was not marked optional, which is a namepath syntax error. The parameter is optional, since `initialize()` defaults it to null. * `wp.customize.addLinkPreviewing()` carried both `@access protected` and `@access private`. The surrounding functions in customize-preview.js use `@access protected`. * The properties of `wp.customize.selectiveRefresh.Placement` were given their types with `@param`, which documents a parameter rather than a member. These become `@member`, as in `wp.customize.Notification`. * The file header of customize-preview.js opened with `/*` rather than `/**`, so its `@output` was not a documentation comment at all. It is now, and the arguments of the function below it move into a docblock of their own, as in the other files here. * Four docblocks in customize-widgets.js were indented with a stray space. Also describe the parameters that were left undescribed in the docblocks this branch already touches, in customize-controls.js, customize-selective-refresh.js and customize-widgets.js. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The JavaScript Documentation Standards give no separator between a parameter name and its description, and none of the examples there use one. Sixty-eight `@param` tags across six of these files did, so they are brought into line and the description column is realigned. Only the separator is removed. Hyphens that belong to a description are left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Thirty-three annotations described a jQuery collection as `{jQuery}`. That
spelling does resolve, which is why nothing complained, but it resolves to
the wrong thing: `jQuery` is declared as a variable of type `JQueryStatic`,
so a reader or a type checker is told these values are the `$` function
itself. The collection type is `JQuery`.
Checked against the repository's own tsconfig.json:
/** @type {jQuery} */ → Property does not exist on type 'JQueryStatic'
/** @type {JQuery} */ → Property does not exist on type 'JQuery<HTMLElement>'
Every one of the thirty-three holds a collection, and several say so in
their own description, such as `@return {jQuery} The jQuery collection.`
in `api.ensure()`. This is the same correction already made for
`jQuery.Promise`, and it leaves the file consistent with the `JQueryStatic`,
`JQuery.Event` and `JQuery.Promise` spellings already in use.
Only the type positions are touched. Prose that talks about a jQuery object
or collection still says jQuery, and the two names are the same length, so
no alignment moves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three overrides of `selectiveRefresh.Partial.prototype.refresh()` documented
their return as `{Promise}`. Each one returns `$.Deferred().promise()`, a
jQuery promise, and the base method they override already documents
`{JQuery.Promise<*>}`. `Promise` names the native constructor, which has a
different API: a jQuery promise carries `done`, `fail` and `always`, and its
`then` predates the Promises/A+ signature.
`WidgetPartial.refresh()` additionally offered `{Promise|void}`. It never
returns void. Both branches return: one the rejected deferred it just built,
the other the result of the base implementation, which itself always returns
its `refreshPromise`.
The descriptions are reworded to say what each promise settles on, since
"A promise postponing the refresh" described the return by its effect on the
caller rather than by what it resolves to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`handleFieldActiveToggle()` was annotated `@this {jQuery}`, but it is passed
to `fieldActiveToggles.each()` and bound with `.on( 'click', … )`, and jQuery
sets `this` to the raw DOM element in both. The function body settles it: it
calls `$( this ).val()` and `$( this ).prop( 'checked' )`, and wrapping would
be pointless if `this` were already a collection.
The elements are the `.hide-column-tog` checkboxes that WP_Screen renders as
`<input type="checkbox">`, so the type is `HTMLInputElement`, which is also
what makes `val()` and the `checked` property meaningful here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`WidgetsPanel` lent its prototype to `wp.customize.Widgets.WigetsPanel`, missing the `d`. The class is spelled `WidgetsPanel` in its own `@class` tag two lines above, in the assignment, and in the panelConstructor registration, so every method in that literal was being documented onto a namespace that exists nowhere else. This is the same slip as the `ThemsPanel` one corrected earlier in this branch. `SidebarPartial` tagged its constructor `@class`. The docblock sits on `initialize`, so `@class` declares `initialize` itself to be the class; `@constructs` is what marks a function as the constructor of the class its prototype is being lent to. Of the nineteen constructor docblocks in these files, this was the only one not using `@constructs` — its sibling `WidgetPartial`, seventy lines earlier in the same file, already did. `api.Value` documented no base class despite extending `api.Class`. Its neighbours in the same file all record theirs: `Values`, `Messenger` and `Notification` augment `wp.customize.Class`, and `Element` augments `wp.customize.Value`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Partial` and `Placement` sit side by side in one file and declared
themselves differently: `Partial` used a bare `@class` and `Placement` wrote
`@class Placement`. That is the only unqualified name given to `@class` in
these files; the other forty-six are either bare alongside `@memberOf`, or
fully qualified without one.
The difference turns out to be load-bearing rather than stylistic. A bare
`@class` makes JSDoc infer the name from the code, and the two chain their
assignments in opposite directions:
Partial = self.Partial = api.Class.extend( … )
self.Placement = Placement = api.Class.extend( … )
So the inference reads `Partial` for the first, which is right, and
`self.Placement` for the second, which yields the longname
`wp.customize.selectiveRefresh.self.Placement`. Simply dropping the name to
match its neighbour would have broken the class.
Both now carry an explicit `@alias`, the form already used for the partial
subclasses in preview-nav-menus.js and preview-widgets.js, so neither
depends on the order its assignment happens to be written in. Running JSDoc
over the file before and after yields the same eighty-eight documented
symbols.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
afercia
left a comment
There was a problem hiding this comment.
The only minor thing I see is that several verbs that start a description should be third person. For example:
- Initialize
- Trigger
- Bind
- Unbind
- Get
- Set
- Update
- Stop
- Link
- Create
- Handle
- Push
- Show
- Hide
- Add
For the rest, I totally defer to you.
|
In other JS files we now have a few occurrences of:
Should we fix them in another PR? |
Yes, let's keep the changes here limited to the Customizer JS files. |
Sixteen descriptions were fragments rather than sentences: some began in lower case and ran on without a period, some restated the parameter name and said nothing, and one described what a parameter accepts by writing out the two literals, `1|-1`, with no indication of what moving by one means. Reviewing the whole set rather than the three that were pointed out, the two shapes turn out to be exhaustive across these files. Every remaining tag description now starts with a capital and ends with a period, and none is left empty where the block already documents the value's siblings. The three offsets are described in terms of what they move, since the caller of `_changeDepth()` cannot tell from `1|-1` that the sign selects a direction rather than an amount, and the function throws for anything else. Only the one prose summary that started in lower case is touched, on the `selectSidebarItem()` helper, where the verb is also put in the third person to match the surrounding blocks. The remaining summaries that lack a closing period are left alone; they are a much larger set and a separate question. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`getInitialHeaderImage()` documented `@return {Object} Options`, which is the
return of `calculateImageSelectOptions()`, the method that sits either side
of it in this file and feeds options to the imgAreaSelect plugin. This one
returns a model: every path ends in `new api.HeaderTool.ImageModel( … )`,
either empty when no header image is set or populated from the matching
upload. Its own summary already said as much, and contradicted the tag
directly below it.
Found while normalizing the wording of the three `@return {Object} Options`
tags, where this one turned out not to belong to that group at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Forty-nine `@param` tags carried a type and a name and stopped there. A reader learning what to pass had only the parameter's own name to go on, and for the ones named `a`, `b`, `args` or `params` that is nothing at all. Where a method aliases or overrides one that is already documented, the existing wording is reused rather than reinvented, so the two now read the same: the `Panel.onChangeExpanded()` and `Control.onChangeActive()` tags come from their counterparts on Section and Container, and the `expand()` and `collapse()` aliases in nav-menus.js and widgets.js from the Container methods they point at. The rest are described from the code. `_children()` is explained in terms of its two call sites, `_children( 'section', 'control' )` and `_children( 'panel', 'section' )`, which show that the first argument names the Value on each child holding its parent's ID while the second names the collection on wp.customize to walk. `setImageFromURL()` and the `widgetId` helpers are described by what their bodies do with the value. No alignment moves: every block already had columns wide enough, so the change is forty-nine lines replaced one for one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Container.onChangeActive()` said its argument was the active state to
"transiution" to.
`Control.onChangeActive()` did not document `args.unchanged`, though the
first thing its body does is branch on it and return early. The override now
documents the same five values as the Container method it mirrors, in the
same order and wording, so the two can be read against each other.
Four arguments were marked required that the code treats as optional. Three
of them, `attachmentId`, `width` and `height` on `setImageFromURL()`, are
each copied into the data object only inside an `if`, and the `options` of
`Value`, `Messenger` and `Previewer` reach `$.extend()` through `options ||
{}`, so omitting any of them is ordinary usage rather than a mistake.
Deliberately left required: `Control.initialize()` reads `options.params ||
options || {}`, which dereferences the argument before the fallback can
apply, so calling it with nothing throws rather than defaulting. For the same
reason `Placement.initialize()` stays required, since the `args || {}` there
only defers the failure to the `args.partial` check that throws two lines
later.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It touches core Customizer runtime modules across many files (not just comments) and warrants human verification against build/tooling expectations and runtime compatibility.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Lite
afercia
left a comment
There was a problem hiding this comment.
Overall, the documentation has greatly improved. LGTM.
For the code changes, I defer to @westonruter
The variadic signatures could not be documented without names for their arguments, so the `arguments` object is replaced with modern rest parameters across `wp.customize.Class`, `Events`, `Value`, `Values`, `Element` and `Messenger`. The `Array.prototype.slice` alias they relied on is no longer needed and is removed. Rest parameters are supported by >96% of users globally and all browser versions supported by WordPress. The argument these two files receive was named `exports`, while the docblock beside the line using it said `window.wp.customize`. It is now `wp`, and the arguments of both wrapping functions are documented. Many of the types named nothing that resolves. Seven tags were typed `mixed`, which is not a JSDoc type; one still carried the `[type]` placeholder an IDE had left behind; and one each read `...`, an unqualified `Value`, and `string|jQuery collection`, which is two words rather than a type expression. Where the jQuery namespace appeared it was spelled in lower case, so it named the `$` function rather than a collection or an event. The value accepted by `Value#set()` was typed as an object, though a `Value` holds anything. In loader.js the one documented parameter had no braces at all. The remaining docblocks are new, since most of these methods carried none. Among what they now record: `Class` returns a function rather than an object whenever the prototype defines an instance method, which is how `wp.customize()` and `wp.customize.control()` come to be callable, and `Value#link()` binds the receiver to follow the values it is given rather than the reverse. QUnit tests cover the applicator form of the constructor, the forwarding of arguments through the event methods, and `unsync()` in both directions, resolving two long-standing todos in that file. Developed as subset of #10743. Follow-up to r48110, r48650. Props westonruter, afercia, grapplerulrich, vishalkakadiya, shailu25. See #39671, #40831, #64662, #66033. git-svn-id: https://develop.svn.wordpress.org/trunk@63454 602fd350-edb4-49c9-b593-d223f7449a82
The variadic signatures could not be documented without names for their arguments, so the `arguments` object is replaced with modern rest parameters across `wp.customize.Class`, `Events`, `Value`, `Values`, `Element` and `Messenger`. The `Array.prototype.slice` alias they relied on is no longer needed and is removed. Rest parameters are supported by >96% of users globally and all browser versions supported by WordPress. The argument these two files receive was named `exports`, while the docblock beside the line using it said `window.wp.customize`. It is now `wp`, and the arguments of both wrapping functions are documented. Many of the types named nothing that resolves. Seven tags were typed `mixed`, which is not a JSDoc type; one still carried the `[type]` placeholder an IDE had left behind; and one each read `...`, an unqualified `Value`, and `string|jQuery collection`, which is two words rather than a type expression. Where the jQuery namespace appeared it was spelled in lower case, so it named the `$` function rather than a collection or an event. The value accepted by `Value#set()` was typed as an object, though a `Value` holds anything. In loader.js the one documented parameter had no braces at all. The remaining docblocks are new, since most of these methods carried none. Among what they now record: `Class` returns a function rather than an object whenever the prototype defines an instance method, which is how `wp.customize()` and `wp.customize.control()` come to be callable, and `Value#link()` binds the receiver to follow the values it is given rather than the reverse. QUnit tests cover the applicator form of the constructor, the forwarding of arguments through the event methods, and `unsync()` in both directions, resolving two long-standing todos in that file. Developed as subset of WordPress/wordpress-develop#10743. Follow-up to r48110, r48650. Props westonruter, afercia, grapplerulrich, vishalkakadiya, shailu25. See #39671, #40831, #64662, #66033. Built from https://develop.svn.wordpress.org/trunk@63454 git-svn-id: http://core.svn.wordpress.org/trunk@62634 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Many of the types named nothing that resolves. The four deferred getters referred to their callback typedefs without a namespace, the promise they return was spelled `jQuery.promise`, which is neither the capitalization the namespace is declared under nor the name of a type, and jQuery events were typed as plain objects. Two blocks documented the wrong thing outright. `ThemesPanel` lent its prototype to a name misspelled without the second `e`, so its methods were attached to a namespace that exists nowhere else, and `getInitialHeaderImage()` carried the return of the method beside it, describing a set of cropper options where its own summary already said it returns a model. Parameters the code treats as optional are marked as such. Developed as subset of #10743. Follow-up to r41799, r48650, r63454. Props westonruter, afercia. See #39671, #39930, #40831, #64662, #66033. git-svn-id: https://develop.svn.wordpress.org/trunk@63455 602fd350-edb4-49c9-b593-d223f7449a82
Many of the types named nothing that resolves. The four deferred getters referred to their callback typedefs without a namespace, the promise they return was spelled `jQuery.promise`, which is neither the capitalization the namespace is declared under nor the name of a type, and jQuery events were typed as plain objects. Two blocks documented the wrong thing outright. `ThemesPanel` lent its prototype to a name misspelled without the second `e`, so its methods were attached to a namespace that exists nowhere else, and `getInitialHeaderImage()` carried the return of the method beside it, describing a set of cropper options where its own summary already said it returns a model. Parameters the code treats as optional are marked as such. Developed as subset of WordPress/wordpress-develop#10743. Follow-up to r41799, r48650, r63454. Props westonruter, afercia. See #39671, #39930, #40831, #64662, #66033. Built from https://develop.svn.wordpress.org/trunk@63455 git-svn-id: http://core.svn.wordpress.org/trunk@62635 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Three blocks described something the code does not do. 1. `WidgetsPanel` lent its prototype to `WigetsPanel`, so its methods were attached to a name that appears nowhere else in core. 2. `WidgetControl.onChangeExpanded()` said its arguments were merged over `defaultActiveArguments`, while the handler that calls it merges over `defaultExpandedArguments`; the two carry separate defaults and separate queues, so the wrong one points a reader at the wrong callback. 3. The field toggle handler in the menu section annotated `this` as a jQuery object, when it is used with `each()` and a click binding, both of which set `this` to the raw element its body then wraps. The `offset` parameters described what they accept by writing out the two literals it may hold, which does not convey that the sign selects a direction rather than an amount. Developed as subset of #10743. Follow-up to r48650, r63454, r63455. Props westonruter, afercia. See #39671, #40831, #64662, #66033. git-svn-id: https://develop.svn.wordpress.org/trunk@63456 602fd350-edb4-49c9-b593-d223f7449a82
Three blocks described something the code does not do. 1. `WidgetsPanel` lent its prototype to `WigetsPanel`, so its methods were attached to a name that appears nowhere else in core. 2. `WidgetControl.onChangeExpanded()` said its arguments were merged over `defaultActiveArguments`, while the handler that calls it merges over `defaultExpandedArguments`; the two carry separate defaults and separate queues, so the wrong one points a reader at the wrong callback. 3. The field toggle handler in the menu section annotated `this` as a jQuery object, when it is used with `each()` and a click binding, both of which set `this` to the raw element its body then wraps. The `offset` parameters described what they accept by writing out the two literals it may hold, which does not convey that the sign selects a direction rather than an amount. Developed as subset of WordPress/wordpress-develop#10743. Follow-up to r48650, r63454, r63455. Props westonruter, afercia. See #39671, #40831, #64662, #66033. Built from https://develop.svn.wordpress.org/trunk@63456 git-svn-id: http://core.svn.wordpress.org/trunk@62636 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Missing tags and descriptions are added, and the types are corrected, whether they named nothing resolvable (`Promise` for `JQuery.Promise<*>`), said too little (`Array` for `string[]`), or were narrower than what the code passes. Both classes in selective-refresh.js lent their prototypes to the wrong name: `Partial` named `wp.customize.SelectiveRefresh.Partial`, capitalized unlike the namespace it lives in, and `Placement` named `wp.customize.selectiveRefresh` itself, so its methods were attached to the module rather than to the class. Developed as subset of #10743. Follow-up to r48110, r48650, r63454, r63455, r63456. Props westonruter, afercia, grapplerulrich, mukesh27. See #39671, #40831, #64662, #66033. git-svn-id: https://develop.svn.wordpress.org/trunk@63457 602fd350-edb4-49c9-b593-d223f7449a82
Missing tags and descriptions are added, and the types are corrected, whether they named nothing resolvable (`Promise` for `JQuery.Promise<*>`), said too little (`Array` for `string[]`), or were narrower than what the code passes. Both classes in selective-refresh.js lent their prototypes to the wrong name: `Partial` named `wp.customize.SelectiveRefresh.Partial`, capitalized unlike the namespace it lives in, and `Placement` named `wp.customize.selectiveRefresh` itself, so its methods were attached to the module rather than to the class. Developed as subset of WordPress/wordpress-develop#10743. Follow-up to r48110, r48650, r63454, r63455, r63456. Props westonruter, afercia, grapplerulrich, mukesh27. See #39671, #40831, #64662, #66033. Built from https://develop.svn.wordpress.org/trunk@63457 git-svn-id: http://core.svn.wordpress.org/trunk@62637 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Neither file documented a single method: only the classes themselves carried a docblock. Every method across the two now says what it does, in the third person, along with the arguments of the function wrapping each file. One behavior is changed: `ChoiceListView.render()` returned nothing, unlike the sibling views beside it, so it now returns `this` as the Backbone convention expects. Nothing reads the value, since the view is only ever constructed, never chained, and `render` is otherwise reached as a `listenTo` handler, whose return is discarded. Developed as subset of #10743. Follow-up to r63454, r63455, r63456, r63457. Props westonruter, afercia, grapplerulrich. See #39671, #64662, #66033. Fixes #40831. git-svn-id: https://develop.svn.wordpress.org/trunk@63458 602fd350-edb4-49c9-b593-d223f7449a82
Neither file documented a single method: only the classes themselves carried a docblock. Every method across the two now says what it does, in the third person, along with the arguments of the function wrapping each file. One behavior is changed: `ChoiceListView.render()` returned nothing, unlike the sibling views beside it, so it now returns `this` as the Backbone convention expects. Nothing reads the value, since the view is only ever constructed, never chained, and `render` is otherwise reached as a `listenTo` handler, whose return is discarded. Developed as subset of WordPress/wordpress-develop#10743. Follow-up to r63454, r63455, r63456, r63457. Props westonruter, afercia, grapplerulrich. See #39671, #64662, #66033. Fixes #40831. Built from https://develop.svn.wordpress.org/trunk@63458 git-svn-id: http://core.svn.wordpress.org/trunk@62638 1a063a9b-81f0-0310-95a4-ce76da25c4cd
✅ Committed in:
Trac ticket: https://core.trac.wordpress.org/ticket/40831
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.