Improve Customizer JSDoc - #10743
Open
westonruter wants to merge 48 commits into
Open
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>
westonruter
commented
Jan 16, 2026
Closed
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>
westonruter
commented
Aug 29, 2026
| }, | ||
|
|
||
| /** | ||
| * Maybe add random choice. |
Member
Author
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 1 comment.
Suppressed comments (5)
src/js/_enqueues/wp/customize/controls.js:6567
- There appears to be a literal tab character used for column padding in this JSDoc line, which is inconsistent with the surrounding alignment and can trip whitespace-sensitive tooling. Replace it with spaces to align the columns.
* @param {string} params.form A selector or jQuery element for the form to be used for POSTing data to the preview frame.
src/js/_enqueues/wp/customize/controls.js:4403
- This param is documented as
{Object}, butrestoreDefault()receives a jQuery event object (it is passed toapi.utils.isKeydownButNotEnterEvent). Use{JQuery.Event}for consistency with other JSDoc in this file.
* @param {Object} event jQuery Event object.
src/js/_enqueues/wp/customize/controls.js:4904
- This param is documented as
{Object}, butremoveFile()receives a jQuery event object (it is passed toapi.utils.isKeydownButNotEnterEvent). Use{JQuery.Event}for consistency with other JSDoc in this file.
* @param {Object} event jQuery Event object.
src/js/_enqueues/wp/customize/controls.js:4418
- This param is documented as
{Object}, butremoveFile()receives a jQuery event object (it is passed toapi.utils.isKeydownButNotEnterEvent). Use{JQuery.Event}for consistency with other JSDoc in this file.
* @param {Object} event jQuery Event object.
src/js/_enqueues/wp/customize/controls.js:4568
- This param is documented as
{Object}, butopenFrame()receives a jQuery event object (it is passed toapi.utils.isKeydownButNotEnterEvent). Use{JQuery.Event}for consistency with other JSDoc in this file.
* @param {Object} event jQuery Event object.
afercia
reviewed
Sep 1, 2026
afercia
reviewed
Sep 1, 2026
afercia
reviewed
Sep 1, 2026
afercia
reviewed
Sep 1, 2026
afercia
reviewed
Sep 1, 2026
Four things, all raised in review: Parameter columns are aligned to the widest type and name within each block, and continuation lines to the description they belong to. Forty-five lines were out, rather than the eight that were pointed at, so the rest were found by lining every block up again. This also removes the tab character sitting among the spaces in the `PreviewFrame` constructor. `jsdoc/check-line-alignment` would have found the same lines, but its fixer also aligns `@param` with `@return` and pulls the return description into the parameter column, which is not how these files or the rest of `src/js` are written. The five handlers taking a jQuery event described it as an Object. Each is bound with `container.on( 'click keydown', … )` and passed to `api.utils.isKeydownButNotEnterEvent()`, which already documents its own argument as `JQuery.Event`. The same applies to the Backbone `click .close` handler in customize-views.js. Finally, the summaries added to customize-models.js and customize-views.js were written in the imperative, where the standards ask for the third person. Rather than only conjugating the verb, each summary now says what the method does, since "Initialize." conveyed little either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The constructor of `NavMenuInstancePartial` documented its options as optional while requiring the `args_hmac` nested inside them, which contradicted itself. The optional markers were the wrong half to keep. Defaulting the arguments to an empty object does not make them optional, it only moves the failure two lines down: an empty object satisfies the `_.isObject()` check and then fails the comparison against the HMAC named in the ID. There is no way to construct one of these partials without supplying them. The route they arrive by is worth stating, so the description now does. In practice they are always the container context, since `WP_Customize_Nav_Menus::filter_wp_nav_menu()` writes them into a `data-customize-partial-placement-context` attribute and emits no `data-customize-partial-options`, leaving `addPartials()` to pass them as `constructingContainerContext`. Passing them in the params is supported but nothing in core does it. The theme location and the menu ID stay optional, both being guarded where they are read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A description too long for one line continues on the next, indented to the column where the description started. Five of them were indented to some other column instead, which reads as a new field rather than a continuation. Four are the `@return` tags on the deferred getters in controls.js, whose continuation sat three columns short. The fifth, on `reflowWidgets()` in preview-widgets.js, was six columns long; since the whole description fits within the width the surrounding docblocks already use, it is joined onto the tag line rather than realigned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`WidgetControl.onChangeExpanded()` documented its `args` as being merged on top of `this.defaultActiveArguments`. It is not: the handler bound in `WidgetControl.initialize()` merges over `defaultExpandedArguments`, the object declared just above it on the same prototype. The active and expanded states carry separate defaults and separate argument queues, so naming the wrong one points a reader at the wrong `completeCallback`. The two members the body actually reads, `args.unchanged` and `args.completeCallback`, are now documented as well, matching the wording `Section.onChangeExpanded()` already uses for the same contract. `args.unchanged` is marked optional because setting `expanded` directly leaves the arguments queue empty, and the defaults do not supply it. `WidgetPartial.renderContent()` called its `placement` "The placement function". A Placement is an object, not a function. It now says what the method it overrides says: the placement to render into. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fifteen `@param` tags carried a type and a name but no description at all, and seven descriptions ended without a period. Both were on lines this branch had already touched, so they were being restated in their incomplete form rather than inherited untouched. The descriptions are taken from what the code does: - The `_toggleExpanded` and `applySavedData` tags in nav-menus.js, and the `_toggleExpanded` tags in widgets.js, alias methods documented elsewhere; they now repeat the wording those methods already carry. The two `applySavedData` payloads list the properties that `WP_Customize_Nav_Menu_Setting::amend_customize_save_response()` and its nav menu item counterpart actually emit, which is why only the nav menu entry mentions `saved_value`. - The arguments to the `rss` form sync handler are named after the `widget-synced` trigger that supplies them. - `_setInputState()` describes its `state` in terms of the three branches the body takes, since what the argument means depends on the input type. - The `partial-content-rendered` handler describes a placement that has already been rendered, not one about to be, because the event fires after the content is in the document. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…into trac-40831
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
reviewed
Sep 2, 2026
afercia
left a comment
afercia
left a comment
Member
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.
Member
|
In other JS files we now have a few occurrences of:
Should we fix them in another PR? |
Member
Author
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.