Improve Customizer JSDoc - #10743
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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/js/_enqueues/wp/customize/base.js:257
wp.customize.Value#set()accepts any value type (strings, numbers, objects, etc.), but the updated JSDoc still documents thetoparameter as{Object}. This makes the inline docs/type checking less accurate.
* Set the value and trigger all bound callbacks.
*
* @param {Object} to New value.
* @param {...*} [args] Zero or more additional arguments to pass to the setter.
* @return {wp.customize.Value} The instance of the Value.
Following the same three cases found in code review, the rest of these files
were checked for parameters whose functions default or guard them but which are
documented as required. Eight more turned up:
* `inherits()` and `wp.customize.Class.extend()` apply `protoProps` and
`staticProps` only `if` they were given, and nearly every subclass in these
files calls `extend()` without the second argument.
* `wp.customize.ThemesSection#updateCount()` counts the visible theme controls
itself when not given a count.
* `wp.customize.Menus~displayNavMenuName()` and the `debounce()` helper in
customize-preview.js default their argument.
* The `Partial` constructors in customize-selective-refresh.js,
customize-preview-widgets.js and customize-preview-nav-menus.js all begin
`options = options || {}`, and each property of those objects is either given
a default or inferred, so those are optional too.
Also `options.fire` in `bindSettingListener()`, which was documented as
required while its parent was already optional, and is only ever read with
`if ( options.fire )`.
Parameters left alone include the `expanded` argument of the various
`onChangeExpanded()` methods, where the `&&` tests the value of a required
boolean rather than whether it was supplied.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A Value holds anything, so `set()` takes anything. It was documented as taking an Object, while the two neighbouring members of the same class already used `*`: `initialize()` for the initial value, and `get()` for what it returns. Also replace two tab characters used to pad a description in customize-controls.js with spaces, so that the column lines up with the rest of its block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🤖 Comment by Claude Opus 5 Noting here that the comment suppressed in the latest review was correct and has been acted on, since suppressed comments are easy to miss. It observed that The review categorised it as "previously missed, in code that hasn't changed since the last review", which undersells it: the line does appear in this pull request's diff, having been reformatted when the rest parameter was added to |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/js/_enqueues/wp/customize/controls.js:6155
- The
callbackargument is optional forwp.customize.control()(it can return a promise when multiple IDs are provided without a callback, as shown in the examples). Mark it as optional in the JSDoc.
This issue also appears on line 6252 of the same file.
* @param {wp.customize.deferredControlsCallback} callback Function called when all supplied controls exist.
src/js/_enqueues/wp/customize/controls.js:6252
- The
callbackargument is optional forwp.customize.panel()(it can return a promise when multiple IDs are provided without a callback, consistent with wp.customize.Values#instance/when usage). Mark it as optional in the JSDoc.
* @param {wp.customize.deferredPanelsCallback} callback Function called when all supplied panels exist.
Supplying several IDs without a callback is a supported way to call these, and returns a promise instead. Each of the two docblocks says as much in its own `@return`, which offers "a promise resolving to requested controls" alongside the instance, so the required `callback` contradicted it. The equivalent accessors for sections and notifications already documented their callback as optional, so all four now agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🤖 Comment by Claude Opus 5 Both comments suppressed in the latest review were correct and have been acted on in 1fa8f87.
So the two halves of the same block disagreed. The equivalent accessors for sections and notifications already had Worth noting for anyone reading along: three of the last four findings on this pull request arrived suppressed, and all three were correct. Two of them, this one included, are on lines that do appear in the diff, and were categorised as "previously missed, in code that hasn't changed since the last review". The common thread is a line that was touched for one reason while a pre-existing problem elsewhere on it went unexamined. |
Four of these are used across the Customizer files and three of them differed: two omitted the dot before the type arguments, and one omitted the space after the comma. `Object.<Key, Value>` is the Closure form, which `jsdoc.conf.json` enables alongside the JSDoc dictionary, and it is what the rest of `src/js` uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On a mobile browser the method navigates with `return window.location = src`,
which returns the URL, so it does not always return nothing. The neighbouring
`beforeunload()` in the same object already documents this shape as
`{string|void}`.
None of the three callers uses the value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
src/js/_enqueues/wp/customize/base.js:90
api.Classmay return a callable function when aninstance()method is present (see theif ( this.instance ) { magic = function( ...instanceArgs ) { ... } }block). The JSDoc currently states it always returns anObject, which is misleading for API consumers and type tooling; includeFunctionin the return type.
* @return {Object} The instance of the class.
src/js/_enqueues/wp/customize/selective-refresh.js:261
- The
placements()method returns actualPlacementinstances created vianew Placement(...). Elsewhere in this file the type is referenced aswp.customize.selectiveRefresh.Placement, so the return type here should be fully-qualified for consistency and to avoid referring to an undefined globalPlacementtype.
* @return {Array.<Placement>} The placements for this partial in the document.
src/js/_enqueues/wp/customize/selective-refresh.js:605
- This description says
removedNodesis a singleElementwhencontainerInclusiveis true, but the implementation assignsplacement.container(a jQuery object) toremovedNodes(and calls.replaceWith()on it). Update the prose to match the actual runtime type and capitalizeDocumentFragmentconsistently.
* If the partial is containerInclusive, then the removedNodes will be
* the single Element that was the partial's former placement. If the
* partial is not containerInclusive, then the removedNodes will be a
* documentFragment containing the nodes removed.
A class whose prototype has an `instance` method is constructed as a function rather than a plain object, so that the instance can be called. That is how `wp.customize.Value` and `wp.customize.Values` are reached as `wp.customize()` and `wp.customize.control()`, and the tests added on this branch cover it, but the constructor was documented as always returning an Object. `Partial#placements()` gave its return as an array of `Placement`, which is the name of a variable inside the closure rather than a documented symbol. The rest of the file refers to `wp.customize.selectiveRefresh.Placement`. Finally, the prose describing `Placement#removedNodes` still said the property holds an Element, which the type beneath it no longer claims, and spelled DocumentFragment in lower case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🤖 Comment by Claude Opus 5 All three comments suppressed in the latest review were correct and have been acted on in 9b65262.
This one is worth dwelling on, because it exposed a hole in the check used earlier on this branch. That check searched for the bare names as whole types, The prose for |
Found by comparing every documented return type against what the function actually returns, and every documented member against what is assigned to it, rather than by checking that the types are well formed. `hidden()` in customize-nav-menus.js finishes with `.get().join( ',' )`, so it returns a string rather than an array. Its one caller sends the value as an Ajax field, which wants the joined form. The `defaultConstructor` of `wp.customize.Values` and of the notifications collection in customize-controls.js each hold a class, and `create()` calls `new this.defaultConstructor()`. Something documented as an Object cannot be constructed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of these already said so in their own descriptions, which ended "Optional." while the parameter was documented as required. `Values#each()` also guards its context with `typeof context === 'undefined'`, a form the earlier pass over optional parameters did not look for, having only searched for defaulting with `||` and guarding with `&&`. `Messenger#send()` guards its data the same way, and is called both with no data at all, as in `send( 'back' )`, and with values that are not objects: a URL string in `handleLinkClick()` and a scroll offset in the preview's scroll handler. So the parameter is optional and holds any type. The now redundant "Optional." is dropped from the two descriptions that had it, since the brackets say the same thing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
afercia
left a comment
afercia
left a comment
There was a problem hiding this comment.
I left only a few minor comments mainly missing docblocks for the params passed to IIFEs, for consistency.
Four of the functions wrapping these files still had their arguments undocumented: those in customize-base.js, customize-loader.js, customize-models.js and customize-preview-nav-menus.js. All eleven files now describe them. In customize-widgets.js the arguments shared a docblock with `@output`, which documents the file rather than the function. They are separated, as elsewhere here. The descriptions in customize-preview-widgets.js also differed from the rest, calling Underscore "the utilities library" and the Customizer API "information from the API". These now read the same in every file. Co-Authored-By: Andrea Fercia <afercia@git.wordpress.org> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The function wrapping this file called its first argument `exports`, and was
given `wp`. Unlike the three other files that had inherited the same name, this
one does use the argument, to publish the API at the end:
exports.customize = api;
That sits directly beneath a docblock reading "Expose the API publicly on
window.wp.customize", which is the name the reader is looking for and not the
one in front of them. Documenting the argument made the mismatch plainer still,
since it had to be described as the WordPress global object while being called
something else.
Renaming makes the last line `wp.customize = api`, and leaves every function
wrapping these files taking the globals under their own names.
Co-Authored-By: Andrea Fercia <afercia@git.wordpress.org>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every other file here opens with the `@output` docblock and declares its globals after it. This one had them the other way round. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Underscore global was documented as an Object, which says nothing about it. `@types/underscore` gives it as `_.UnderscoreStatic`, and that name does resolve: a check of the three spellings under `tsconfig.json` accepts `_.UnderscoreStatic` and rejects a bare `UnderscoreStatic`. Nine promise types spelled `jQuery.promise` in lower case. `Promise` is the name of a type, not of the `promise()` method, and the other five occurrences already had it capitalised. Neither `jQuery.Promise` nor `jQuery.promise` resolves under TypeScript, which wants `JQuery.Promise` with its type arguments. That is a larger change and one for whenever these files are added to `tsconfig.json`; this only makes the existing spelling consistent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`@types/jquery` puts its types under `JQuery`, so the lower case `jQuery.Event` and `jQuery.jqXHR` name a namespace that does not exist, and `jQuery.Promise` does the same while also omitting the type argument its interface requires. This branch had already adopted `JQueryStatic` for the global itself, leaving the two spellings side by side in the same docblocks. Thirty-one types are updated: sixteen events, fourteen promises, and one jqXHR. Each form was checked against `tsconfig.json` first, along with the ones left alone. `jQuery` on its own does resolve, so the collection type is untouched. `Array` used without a type argument remains, in twenty-two places. Unlike these, the element type has to be read out of each function rather than substituted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Twenty-two array types gave no element type, which TypeScript rejects outright and which tells a reader nothing. Each was read out of the code rather than inferred from the parameter name: * Setting, widget, theme location and query parameter names, search terms and tags are all arrays of strings. * `_children()` collects panels, sections or controls depending on its arguments, so it returns instances of the base class, while `sections()` and `controls()` return the type they name. * The lists compared by `areElementListsEqual()` are gathered with `_.pluck( …, 'headContainer' )` and hold jQuery objects rather than elements. * Theme data, nav menu updates and CSSLint annotations are plain objects, and `HeaderTool.CombinedList` is constructed from Backbone collections. * `_getInputState()` returns an array only for a multiple select, whose selected options give strings. Four types already naming their element used `Array.<Type>` where eleven others used `Type[]`, so those are brought over too. The one left is an array of a record type, where the brackets would nest awkwardly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🤖 Comment by Claude Opus 5 Following on from the review, the types touched while documenting those arguments were checked against The jQuery namespace is Thirty-one occurrences are updated. Underscore and the promise casing (ffa1007). Nine promise types were spelled Arrays now say what they contain (4f8350c). Twenty-two gave no element type. Each was read out of the function rather than guessed from the parameter name, which mattered in at least two places: the lists compared by Worth noting for anyone weighing this against #13251: the capital-J spelling is the correct one there too, and the same three namespace members appear a further eleven times elsewhere 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.