Develop to main sync for v2.2.0 release - #2095
Open
KMchaudhary wants to merge 19 commits into
Open
Conversation
* fix: resync package-lock.json so npm ci works again `npm ci` has been failing on every pull request, which takes down both the JS unit tests and the Plugin Check workflow before either does any real work: npm error `npm ci` can only install packages when your package.json and npm error package-lock.json are in sync. npm error Missing: @opentelemetry/core@2.2.0 from lock file npm error Missing: markdown-it@12.3.2 from lock file ... and ~40 more The lock had drifted from the dependency tree it is supposed to describe: 42 transitive packages were absent, and markdown-it was recorded as 14.2.0 hoisted at the top level when its only consumer, markdownlint 0.25.1 (a dev tool pulled in by @wordpress/scripts), requires 12.3.2. `npm install` regenerates it correctly, nesting markdown-it 12.3.2 under markdownlint with its matching linkify-it, mdurl and uc.micro. Nothing is upgraded by this. Comparing the two lock files package by package: 42 added, 6 removed, and zero version changes to any package that was already locked. The six removals are the incorrectly hoisted markdown-it and its dependencies, which reappear nested at the versions their consumer asks for. package.json is untouched. Verified locally, running exactly what the two failing workflows run: npm ci exit 0 (2617 packages; previously EUSAGE) npm run test:unit exit 0 (8 suites, 96 tests) npm run build:prod exit 0 (no errors) This is a pre-existing breakage rather than a regression from any recent branch: the same failure reproduces against an untouched checkout of develop. * fix: stop shipping the PHP test suite in the distribution build Plugin Check fails with one error: FILE: tests/php/AddonDependencyMessagesTest.php ERROR missing_direct_file_access_protection .distignore excluded phpunit.xml.dist and the JS tests but never the tests/ directory itself, so all seven PHP test files were rsynced into the plugin ZIP. Plugin Check runs against that unzipped ZIP, so it scanned them. AddonDependencyMessagesTest.php is the only one with a top-level require_once — executable code at file scope with no ABSPATH guard — hence the single error. The other five are pure namespace and class declarations with no side effects, and tests/bootstrap.php defines ABSPATH itself, which is why neither tripped it. The error is not new. It arrived with the #465 fix in 25f58d3 and was masked because npm ci failed earlier in the same job, so Plugin Check never reached the check. Fixing the lock in this branch is what exposed it. Excluding tests/ is the right fix rather than adding an ABSPATH guard: the suite is not usable from the ZIP anyway (phpunit.xml.dist is already excluded) and a guard would exit under the PHPUnit CLI. The pattern is anchored so it means this plugin's suite and not vendor/**/tests/. Verified by running the real Plugin Check against distribution builds of both revisions, under the genuine godam slug so text-domain checks behave as in CI: before 1 error tests/php/AddonDependencyMessagesTest.php after 0 errors The rsync -rc --exclude-from=.distignore the deploy action performs was reproduced locally: the delta is exactly the seven test files, nothing else. composer phpunit still passes (55 tests, 125 assertions) — .distignore has no bearing on the repo checkout that CI tests from.
* fix: restore built-file references in the translation template
WordPress resolves JS translations by hashing the *enqueued* script path:
_load_script_textdomain_from_src() looks for
languages/godam-<locale>-md5( 'assets/build/js/media-library.min.js' ).json,
having first stripped '.min' ("Translations are always based on the unminified
filename"). translate.wordpress.org derives those filenames from the POT's own
file references. So a JS string whose only reference is under assets/src can
never be resolved — that path is not what gets enqueued, and .distignore strips
it from the release anyway.
Excluding assets/build from make-pot did exactly that. Measured on the POT
currently in develop: 1268 entries lost their built-file reference, every one of
the 47 built files the shipped POT referenced went to 0, and 407 entries were
left with a .js reference only under assets/src. Every string behind the 17
wp_set_script_translations() calls silently stopped translating.
Scanning both trees is necessary but not sufficient: '*.min.js' is in make-pot's
hard-coded exclude list, so a plain scan still skips most of assets/build. An
explicit --include="assets/build" outranks it, because IterableCodeExtractor
scores an include by path depth (2) against a single-segment exclude (1). Hence
two passes, merged — built references for the runtime, source references and
translator comments for humans. bin/make-pot.sh carries the reasoning, and it
refuses to run without assets/build rather than silently producing a POT with no
built references, which is how this went unnoticed.
Verified end to end by generating a fr_FR PO from the new POT and running the
real `wp i18n make-json`, then checking each filename against the md5 core
computes:
built JS bundles referenced 38
resolvable at runtime 38 (develop: 0)
Against the last shipped POT: 0 built-file references lost, 3 gained, 47 strings
gained, 3 dropped. The 3 are stale — they exist in neither assets/src nor
assets/build, so they were left over from an older build.
* fix: guard the form-field hooks and dependency reads that moved to init
Three follow-ons from moving translation off `plugins_loaded` in #2057.
WPForms and Fluent Forms field construction moved to `init`, which is later than
the `wpforms_loaded` / `fluentform/loaded` hooks it replaced. Those hooks carried
a guarantee `init` does not: the form plugin had booted, so its classes existed.
On `init` the only remaining gate is `is_plugin_active()`, which stays true when
a plugin is listed in active_plugins but aborted its own bootstrap — a failed
requirements check, a license bail, fatal-recovery mode. In that state
WPForms_Field_GoDAM_Video is never declared (its file declares the class only
inside `if ( class_exists( 'WPForms_Field' ) )`) and Recorder_Field cannot load at
all, because it does `extends BaseFieldManager` unconditionally. Either one is a
fatal on every request, front end included. Both call sites now check first.
Abstract_Addon read $dep['check'] without isset() in both loops, while the same
loops had just gained `$dep['message'] ?? ''`. An entry with only name + message —
an easy assumption now that message is optional — emits `Undefined array key
"check"` from every plugins_loaded, twice per add-on, which under
WP_DEBUG_DISPLAY prints into the page: the symptom class #465 was about. An entry
with no usable check still counts as satisfied, as before.
The media list table's sortable-columns filter was left registered at file scope
while the other two moved to admin_init, so it ran on front-end, REST and cron
requests, and advertised convert_status as sortable even when the API key check
had declined to add the column. All three now register together, and the key
check moved into the callbacks — so admin_init no longer resolves the API key
(potentially an HTTP call) on every admin request including admin-ajax.php, for
a column that renders on one screen.
Tests: the two source-shape guards were pinned so tightly that any future
add_action( 'wpforms_loaded', … ) in that file would fail CI; they now name the
callback they care about. Added guards for the class_exists checks and for all
three column hooks, plus two behavioural tests for the malformed dependency
entry — phpunit.xml.dist converts warnings to exceptions, so an unguarded read
fails rather than passing quietly. Each new guard was mutation-checked: reverting
the fix it covers turns it red.
60 tests, 139 assertions. phpcs clean.
* chore: date 2.1.1 and correct two entries this branch invalidated
The release date moves to August 6, 2026 in both CHANGELOG.md and readme.txt.
Two entries described behaviour that no longer matches the code:
"built from source instead of compiled assets" was the bug, not the fix — that
is what dropped every built-file reference and stopped JS strings translating.
The template now covers both trees.
The API key is no longer re-verified on any admin request either, not just
front-end/REST/cron: the check moved into the column callbacks, so it happens on
the media library screen and nowhere else.
* chore: inline the pot script instead of a shell wrapper
Two composer commands rather than bin/make-pot.sh. Same output — verified
byte-identical apart from the POT-Creation-Date header — and the reasoning lives
in scripts-descriptions where composer will show it.
Dropped the assets/build presence check with it. Without a build, pass 1 finds
nothing to merge and the POT quietly comes out with source references only,
which is the failure this whole change is about, so the script description says
to build first.
* chore: write the 2.1.1 changelog for users, not for us
Dropped "PHP unit tests now run on every pull request". readme.txt is what
wordpress.org shows someone deciding whether to update, and our CI has no
bearing on their site. The v2.1.0 "Chore:" entry earns its place by being about
security advisories; this one was about us.
Restated the rest as what a site owner sees. The i18n fix is a debug log that
stops filling up, not a set of call sites moved off an action. The API key change
is fewer outbound requests on page views, not an HTTP call relocated between
hooks.
"47 strings" is now 34. 47 is the msgid delta against the 2.1.0 tag, but 13 of
those come only from assets/src/blocks/godam-player/track-uploader.js, which
nothing in the repo imports — so they are extractable, not translatable, and no
user will ever see them. The remaining 34 are real player UI: transcript,
chapters, comments, sharing, sign-in.
Also restored assets/build to pass 2's exclude in the pot script, per review.
Pass 1's --include already covers build, so this only saves a redundant scan:
the generated POT is byte-identical either way, which is why languages/ is
unchanged here.
* fix: correct the media-column docblock, unbundle the workflow comment
The docblock claimed all three hooks "defer to rtgodam_is_api_key_valid() there".
Two do — manage_media_columns and manage_upload_sortable_columns, the pair that
decide whether the column exists. rtgodam_add_status_columns_content() does not,
and does not need to: manage_media_custom_column only fires for a column
manage_media_columns already returned.
The head.sha comment is out. Whether these workflows should test the pushed
commit or the merge result is its own question, and arguing for it in a comment
here bundles that decision into a translation fix. Handled separately.
* test: make the #465 guards behavioural instead of source regexes
The old assertions read the source and matched patterns against it, which was
wrong in both directions. `add_action( 'wpforms_loaded', … )` appearing anywhere
in that file failed CI even if unrelated, while adding a fresh `__()` inside
`boot_addons()` reintroduced the bug with all five assertions still green.
tests/stubs/hooks.php is a small Plugin API: registration is recorded, and a hook
can be run on demand. tests/stubs/i18n.php records every translation call. Between
them a test can run `plugins_loaded` and assert nothing translated while it did,
which covers the property rather than four known call sites.
The suite now drives the real path: construct Addon_Registry so its constructor
registers on plugins_loaded, register an add-on whose dependency is unmet and
whose message translates, fire plugins_loaded, assert the translation log is
empty, then fire admin_notices and assert the text appears. Same for the
version-compatibility notice, the admin-only guard, the media column registration
(by loading the transcoder functions file and reading the registry), and both form
integrations — including that each survives its host plugin being in
active_plugins without having bootstrapped, which is the fatal the class_exists
guards prevent.
Verified by mutation. Every one of these is caught:
fresh __() inside boot_addons() 3 failures
WPForms field moved back to wpforms_loaded 1 failure
WPForms class_exists guard removed 1 error
Fluent Forms class_exists guard removed 1 error
sortable filter dropped from the registration 1 failure
dependency message resolved during plugins_loaded 2 failures
The first is the one the old tests let through.
Also drops the file-scope require from AddonDependencyMessagesTest now that the
bootstrap loads the class — that require was what tripped Plugin Check's
missing_direct_file_access_protection while tests were still shipping.
* ci: test the merge result, not the pushed commit (#2065)
Both test workflows pinned `ref: github.event.pull_request.head.sha`, which opts
out of checkout's default `refs/pull/N/merge`. They therefore tested the branch as
written rather than as it will exist on the base, so a change landing on develop
that breaks a branch stayed green on that branch's PR and only failed later, on
the release PR into main.
Removing the override restores the default. plugin_check_on_pull_request.yml
already checks out the merge ref, so this makes the three consistent.
phpcs_on_pull_request.yml keeps its override deliberately: it posts inline review
comments, which have to anchor to commits that exist on the pull request.
* fix: regenerate the POT from a clean build
The committed POT was generated against a working assets/build that had drifted
from what `npm ci && npm run build:prod` produces from scratch, so it recorded
3276 build reference lines across 1551 distinct positions where a clean build
yields 1744 across 77. The Translation Template job caught it on the first run,
which is what the job is for.
It also still carried the pre-docblock line numbers for
admin/godam-transcoder-functions.php — 464/473 rather than 466/475 — because that
file gained two comment lines after the POT was last generated.
Regenerated after `rm -rf assets/build && npm ci && npm run build:prod`. Both
figures now match what CI produced: 1744 build reference lines, and 466/475 for
the transcoder references.
* fix: stop translating during plugins_loaded (_load_textdomain_just_in_time) (#2057) * Resync package-lock.json to fix npm ci failures (#2062) * fix: resync package-lock.json so npm ci works again `npm ci` has been failing on every pull request, which takes down both the JS unit tests and the Plugin Check workflow before either does any real work: npm error `npm ci` can only install packages when your package.json and npm error package-lock.json are in sync. npm error Missing: @opentelemetry/core@2.2.0 from lock file npm error Missing: markdown-it@12.3.2 from lock file ... and ~40 more The lock had drifted from the dependency tree it is supposed to describe: 42 transitive packages were absent, and markdown-it was recorded as 14.2.0 hoisted at the top level when its only consumer, markdownlint 0.25.1 (a dev tool pulled in by @wordpress/scripts), requires 12.3.2. `npm install` regenerates it correctly, nesting markdown-it 12.3.2 under markdownlint with its matching linkify-it, mdurl and uc.micro. Nothing is upgraded by this. Comparing the two lock files package by package: 42 added, 6 removed, and zero version changes to any package that was already locked. The six removals are the incorrectly hoisted markdown-it and its dependencies, which reappear nested at the versions their consumer asks for. package.json is untouched. Verified locally, running exactly what the two failing workflows run: npm ci exit 0 (2617 packages; previously EUSAGE) npm run test:unit exit 0 (8 suites, 96 tests) npm run build:prod exit 0 (no errors) This is a pre-existing breakage rather than a regression from any recent branch: the same failure reproduces against an untouched checkout of develop. * fix: stop shipping the PHP test suite in the distribution build Plugin Check fails with one error: FILE: tests/php/AddonDependencyMessagesTest.php ERROR missing_direct_file_access_protection .distignore excluded phpunit.xml.dist and the JS tests but never the tests/ directory itself, so all seven PHP test files were rsynced into the plugin ZIP. Plugin Check runs against that unzipped ZIP, so it scanned them. AddonDependencyMessagesTest.php is the only one with a top-level require_once — executable code at file scope with no ABSPATH guard — hence the single error. The other five are pure namespace and class declarations with no side effects, and tests/bootstrap.php defines ABSPATH itself, which is why neither tripped it. The error is not new. It arrived with the #465 fix in 25f58d3 and was masked because npm ci failed earlier in the same job, so Plugin Check never reached the check. Fixing the lock in this branch is what exposed it. Excluding tests/ is the right fix rather than adding an ABSPATH guard: the suite is not usable from the ZIP anyway (phpunit.xml.dist is already excluded) and a guard would exit under the PHPUnit CLI. The pattern is anchored so it means this plugin's suite and not vendor/**/tests/. Verified by running the real Plugin Check against distribution builds of both revisions, under the genuine godam slug so text-domain checks behave as in CI: before 1 error tests/php/AddonDependencyMessagesTest.php after 0 errors The rsync -rc --exclude-from=.distignore the deploy action performs was reproduced locally: the delta is exactly the seven test files, nothing else. composer phpunit still passes (55 tests, 125 assertions) — .distignore has no bearing on the repo checkout that CI tests from. * Fix built-file references and guard form-field hooks in init (#2063) * fix: restore built-file references in the translation template WordPress resolves JS translations by hashing the *enqueued* script path: _load_script_textdomain_from_src() looks for languages/godam-<locale>-md5( 'assets/build/js/media-library.min.js' ).json, having first stripped '.min' ("Translations are always based on the unminified filename"). translate.wordpress.org derives those filenames from the POT's own file references. So a JS string whose only reference is under assets/src can never be resolved — that path is not what gets enqueued, and .distignore strips it from the release anyway. Excluding assets/build from make-pot did exactly that. Measured on the POT currently in develop: 1268 entries lost their built-file reference, every one of the 47 built files the shipped POT referenced went to 0, and 407 entries were left with a .js reference only under assets/src. Every string behind the 17 wp_set_script_translations() calls silently stopped translating. Scanning both trees is necessary but not sufficient: '*.min.js' is in make-pot's hard-coded exclude list, so a plain scan still skips most of assets/build. An explicit --include="assets/build" outranks it, because IterableCodeExtractor scores an include by path depth (2) against a single-segment exclude (1). Hence two passes, merged — built references for the runtime, source references and translator comments for humans. bin/make-pot.sh carries the reasoning, and it refuses to run without assets/build rather than silently producing a POT with no built references, which is how this went unnoticed. Verified end to end by generating a fr_FR PO from the new POT and running the real `wp i18n make-json`, then checking each filename against the md5 core computes: built JS bundles referenced 38 resolvable at runtime 38 (develop: 0) Against the last shipped POT: 0 built-file references lost, 3 gained, 47 strings gained, 3 dropped. The 3 are stale — they exist in neither assets/src nor assets/build, so they were left over from an older build. * fix: guard the form-field hooks and dependency reads that moved to init Three follow-ons from moving translation off `plugins_loaded` in #2057. WPForms and Fluent Forms field construction moved to `init`, which is later than the `wpforms_loaded` / `fluentform/loaded` hooks it replaced. Those hooks carried a guarantee `init` does not: the form plugin had booted, so its classes existed. On `init` the only remaining gate is `is_plugin_active()`, which stays true when a plugin is listed in active_plugins but aborted its own bootstrap — a failed requirements check, a license bail, fatal-recovery mode. In that state WPForms_Field_GoDAM_Video is never declared (its file declares the class only inside `if ( class_exists( 'WPForms_Field' ) )`) and Recorder_Field cannot load at all, because it does `extends BaseFieldManager` unconditionally. Either one is a fatal on every request, front end included. Both call sites now check first. Abstract_Addon read $dep['check'] without isset() in both loops, while the same loops had just gained `$dep['message'] ?? ''`. An entry with only name + message — an easy assumption now that message is optional — emits `Undefined array key "check"` from every plugins_loaded, twice per add-on, which under WP_DEBUG_DISPLAY prints into the page: the symptom class #465 was about. An entry with no usable check still counts as satisfied, as before. The media list table's sortable-columns filter was left registered at file scope while the other two moved to admin_init, so it ran on front-end, REST and cron requests, and advertised convert_status as sortable even when the API key check had declined to add the column. All three now register together, and the key check moved into the callbacks — so admin_init no longer resolves the API key (potentially an HTTP call) on every admin request including admin-ajax.php, for a column that renders on one screen. Tests: the two source-shape guards were pinned so tightly that any future add_action( 'wpforms_loaded', … ) in that file would fail CI; they now name the callback they care about. Added guards for the class_exists checks and for all three column hooks, plus two behavioural tests for the malformed dependency entry — phpunit.xml.dist converts warnings to exceptions, so an unguarded read fails rather than passing quietly. Each new guard was mutation-checked: reverting the fix it covers turns it red. 60 tests, 139 assertions. phpcs clean. * chore: date 2.1.1 and correct two entries this branch invalidated The release date moves to August 6, 2026 in both CHANGELOG.md and readme.txt. Two entries described behaviour that no longer matches the code: "built from source instead of compiled assets" was the bug, not the fix — that is what dropped every built-file reference and stopped JS strings translating. The template now covers both trees. The API key is no longer re-verified on any admin request either, not just front-end/REST/cron: the check moved into the column callbacks, so it happens on the media library screen and nowhere else. * chore: inline the pot script instead of a shell wrapper Two composer commands rather than bin/make-pot.sh. Same output — verified byte-identical apart from the POT-Creation-Date header — and the reasoning lives in scripts-descriptions where composer will show it. Dropped the assets/build presence check with it. Without a build, pass 1 finds nothing to merge and the POT quietly comes out with source references only, which is the failure this whole change is about, so the script description says to build first. * chore: write the 2.1.1 changelog for users, not for us Dropped "PHP unit tests now run on every pull request". readme.txt is what wordpress.org shows someone deciding whether to update, and our CI has no bearing on their site. The v2.1.0 "Chore:" entry earns its place by being about security advisories; this one was about us. Restated the rest as what a site owner sees. The i18n fix is a debug log that stops filling up, not a set of call sites moved off an action. The API key change is fewer outbound requests on page views, not an HTTP call relocated between hooks. "47 strings" is now 34. 47 is the msgid delta against the 2.1.0 tag, but 13 of those come only from assets/src/blocks/godam-player/track-uploader.js, which nothing in the repo imports — so they are extractable, not translatable, and no user will ever see them. The remaining 34 are real player UI: transcript, chapters, comments, sharing, sign-in. Also restored assets/build to pass 2's exclude in the pot script, per review. Pass 1's --include already covers build, so this only saves a redundant scan: the generated POT is byte-identical either way, which is why languages/ is unchanged here. * fix: correct the media-column docblock, unbundle the workflow comment The docblock claimed all three hooks "defer to rtgodam_is_api_key_valid() there". Two do — manage_media_columns and manage_upload_sortable_columns, the pair that decide whether the column exists. rtgodam_add_status_columns_content() does not, and does not need to: manage_media_custom_column only fires for a column manage_media_columns already returned. The head.sha comment is out. Whether these workflows should test the pushed commit or the merge result is its own question, and arguing for it in a comment here bundles that decision into a translation fix. Handled separately. * test: make the #465 guards behavioural instead of source regexes The old assertions read the source and matched patterns against it, which was wrong in both directions. `add_action( 'wpforms_loaded', … )` appearing anywhere in that file failed CI even if unrelated, while adding a fresh `__()` inside `boot_addons()` reintroduced the bug with all five assertions still green. tests/stubs/hooks.php is a small Plugin API: registration is recorded, and a hook can be run on demand. tests/stubs/i18n.php records every translation call. Between them a test can run `plugins_loaded` and assert nothing translated while it did, which covers the property rather than four known call sites. The suite now drives the real path: construct Addon_Registry so its constructor registers on plugins_loaded, register an add-on whose dependency is unmet and whose message translates, fire plugins_loaded, assert the translation log is empty, then fire admin_notices and assert the text appears. Same for the version-compatibility notice, the admin-only guard, the media column registration (by loading the transcoder functions file and reading the registry), and both form integrations — including that each survives its host plugin being in active_plugins without having bootstrapped, which is the fatal the class_exists guards prevent. Verified by mutation. Every one of these is caught: fresh __() inside boot_addons() 3 failures WPForms field moved back to wpforms_loaded 1 failure WPForms class_exists guard removed 1 error Fluent Forms class_exists guard removed 1 error sortable filter dropped from the registration 1 failure dependency message resolved during plugins_loaded 2 failures The first is the one the old tests let through. Also drops the file-scope require from AddonDependencyMessagesTest now that the bootstrap loads the class — that require was what tripped Plugin Check's missing_direct_file_access_protection while tests were still shipping. * ci: test the merge result, not the pushed commit (#2065) Both test workflows pinned `ref: github.event.pull_request.head.sha`, which opts out of checkout's default `refs/pull/N/merge`. They therefore tested the branch as written rather than as it will exist on the base, so a change landing on develop that breaks a branch stayed green on that branch's PR and only failed later, on the release PR into main. Removing the override restores the default. plugin_check_on_pull_request.yml already checks out the merge ref, so this makes the three consistent. phpcs_on_pull_request.yml keeps its override deliberately: it posts inline review comments, which have to anchor to commits that exist on the pull request. * fix: regenerate the POT from a clean build The committed POT was generated against a working assets/build that had drifted from what `npm ci && npm run build:prod` produces from scratch, so it recorded 3276 build reference lines across 1551 distinct positions where a clean build yields 1744 across 77. The Translation Template job caught it on the first run, which is what the job is for. It also still carried the pre-docblock line numbers for admin/godam-transcoder-functions.php — 464/473 rather than 466/475 — because that file gained two comment lines after the POT was last generated. Regenerated after `rm -rf assets/build && npm ci && npm run build:prod`. Both figures now match what CI produced: 1744 build reference lines, and 466/475 for the transcoder references. --------- Co-authored-by: Kuldip Chaudhary <64731232+KMchaudhary@users.noreply.github.com> Co-authored-by: Subodh Rajpopat <subodh.rajpopat@rtcamp.com>
* feat: add filter hooks for Video Thumbnails guide message in media popup and godam/video block
* fix: show transcoding status and retranscode on media grid when folder organization is disabled
* feat: add 'Play on modal' option for the GoDAM video player
Add a 'Play on modal' toggle (default false) to the godam/video block,
[godam_video] shortcode, Elementor widget, and WPBakery element. When
enabled, clicking the inline player opens it in a lightbox modal
(inspired by the shoppable video modal) and plays it there.
* feat: add lightbox element triggers and deep linking
Rename the video option 'Play on modal' to 'Show in lightbox', and make a lightbox view addressable so it can be opened from anywhere and shared.
- Any element with `data-godam-lightbox="{id}"`, and any link to `#godam-video-{id}`, opens the lightbox. A video that is not rendered on the page falls back to its embed page in an iframe
- `#godam-video-{id}` URLs open the lightbox on load, so a link shared from inside it reopens it. The URL stays in step with the lightbox for the life of the page, and is cleared on close
- Share links use the WordPress attachment ID, falling back to the job ID for virtual media. GoDAM Central and embed links are unchanged
- Add `GoDAMAPI.openLightbox()`, `GoDAMAPI.closeLightbox()`, and `GoDAMAPI.isLightboxOpen()`
- Load the player runtime on pages whose only GoDAM content is a trigger, with the `rtgodam_enqueue_lightbox_runtime` filter as an escape hatch
- Fix the share modal opening behind the lightbox overlay, ModalManager being constructed per PlayerManager and producing duplicate body-level overlays, and Tab escaping the open dialog
* Resolve the PR feedback
* fix: address lightbox re-review feedback
* fix: make the lightbox work on touch and iOS devices
---------
Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
* feat(analytics): emit a viewed impression per hotspot Hotspots emitted only hovered/clicked; the layer emitted one viewed that every hotspot inherited. That inheritance is wrong the moment hotspots are added or removed, and it cannot be corrected after the fact because the analytics rollup buckets by day, so a same-day edit is unrecoverable. Each hotspot now emits its own viewed when the layer becomes visible, keyed by the composite id the pipeline already understands. No schema change. Batched to keep the cost flat: addLayerInteractions() writes the layer event and all hotspot events in one sessionStorage round trip, so an impression costs the same as when only the layer event fired. Falls back to individual writes if the batch writer is unavailable. * docs(analytics): record why hotspots with no activity in range are not listed The hotspot list is built from recorded activity in the selected range, not from the layer's saved configuration, so a hotspot absent from the range does not render at all. That is a product decision, not an oversight: a hotspot that was not present during the range has nothing to report, and showing it would invite comparison against hotspots that were actually live. Documented in place so it is not later 'fixed' by enumerating from the config. * fix(layer-analytics): show a hotspot's own viewed, not the layer's The dashboard hook overwrote each hotspot's viewed with the parent layer's, so the fix stopped at the API boundary: the service returned the correct per-hotspot number and the screen threw it away. A hotspot added after the layer had been seen 8 times still rendered 9 views with 9 charged to No Action, which is the originally reported bug. Its hovered and clicked were already correct, which is what made it easy to miss. Both comments justifying the override said hotspots do not emit their own viewed. They do now. Three tests over groupRows; two of them fail against the old code. * test(layer-analytics): assert Woo product hotspots take the same path groupRows is shared between hotspot and woo, but layerType selects the No Action formula, so a woo-only regression would slip past the hotspot-typed assertions. * fix(player): address review — batch-only emission, safe fallback, accurate docstring - emitHotspotEvent / emitParentLayerEvent: allow build-only mode (a `collector` array) to run even when window.GoDAM.addLayerInteraction is absent; only the direct-write path needs it (Copilot). This lets batched emission work against a bundle/mock that provides only addLayerInteractions. - emitLayerVisible: guard the single-write fallback so it never calls a missing addLayerInteraction (would otherwise throw once the guard above is relaxed and no writer exists). - storage.js addLayerInteractions JSDoc: state the fields actually enforced (layer_id, layer_type, action_type); layer_timestamp is expected downstream but not enforced since 0 is valid (Copilot). - Tests: batches with only the batch writer present; does not throw with no writer at all. * docs(player): correct stale dwell comment; hotspots now emit their own viewed * fix(analytics): address combined review (player + dashboard) - Funnel: clamp the per-action share to 100% so a hotspot with hovered > its own viewed (migration-window or dropped-view data) no longer renders a bar overflowing its fixed-height track or a ">100% of viewers" label. Counts row keeps the raw numbers. - emitLayerVisible: require a writer up front. The emit* helpers mark the per-session dedupe as they build, so building with no sink burned the key and the next visibility emitted nothing; bailing early preserves the retry. - emitLayerVisible: skip the per-hotspot `viewed` for a hotspot with no stable id, whose composite key would fall back to positional idx<n> and re-attribute across a deletion now that `viewed` is the denominator. - flushLayerInteractions: chunk keepalive POSTs by a 32 KiB byte budget, not a fixed 100-event count, so per-hotspot volume can't push a request past the shared ~64 KiB keepalive limit and fail silently; swallow the keepalive rejection. Adds tests for the no-writer retry and the id-less skip.
* Fix: media library folder operations breaking in wp.media selector popup Mount the folder sidebar into the opened frame's own menu (retrying until it renders) and hand React the exact root, instead of guessing the last visible frame. Bind mounting to the frame event so reused frames restore the sidebar on reopen. Gate the modal-close reset so it only runs once the last media modal closes, so a nested-modal close no longer wipes the active folder selection mid-use. * Fix/godam media library issues (#2060) * Fix: [point-1] Folder filter issue on media modal * Fix: Folder delete reports success even when the server rejects it * Fix: New Folder button nesting under the last right-clicked folder * Fix: Folder locked state * Fix: media folder zip issues * Fix: IDOR on folder assignment * Fix: Media Library JS bundles load on every admin page * - wp_cache_flush() → wp_cache_flush_runtime() in the ZIP builder - consolidated the three modal-close detectors down to the single wp.media Modal.close() hook * Fix: ZIP builds in the background instead of blocking the request * Fix: [Bug] Media Library correctness cleanup * Address PR #2060 review: point-7 screens, lock gaps, async-ZIP polling, correctness nits * Fix: Media Library Delete folder issue (#2081) * Fix: Media Library Delete folder issue * Resolve Copilot and other feedbacks --------- Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com> Co-authored-by: Rudrakshi Gupta <109859631+rudrakshi-gupta@users.noreply.github.com>
…yer (#2084) The inline <style> rendered as a sibling of the player markup, so in a flow-layout container (core/column, core/group) the player became the second child and picked up the container's blockGap margin-block-start, adding phantom spacing above the video. The CSS is static, so register it as a stylesheet and hang it off godam-player-style as a dependency. Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
* fix(gallery): move modal close button outside the video area * fix(lightbox): show only the play icon on a closed lightbox poster * Resolve the custom transcript not reflecting on frontend issue * fix(gallery): adjust close button alignment for notched devices and improve modal height handling --------- Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
… odt/ods/odp, txt and csv. (#2079) * feat: support Office, OpenDocument and text files in the Document block Extend the Document block beyond PDF to docx/doc, xlsx/xls, pptx/ppt, odt/ods/odp, txt and csv. These are sent to GoDAM Central as job_type=document, and the preview PDF it returns is stored in new rtgodam_preview_pdf_url meta. - Render every document through a chrome-less pdf.js viewer (react-pdf), loaded as a lazy chunk. Downloads always serve the original upload, never the generated preview. - Add rtgodam_get_supported_document_types() as the single source of truth, mirrored in the editor by constants.js and kept in sync by test. - Classify documents by MIME *and* extension: text/plain also covers .srt/.asc/.c/.cc/.h, which have no conversion path and would otherwise be dispatched as video transcodes and show a stuck progress spinner. - Extend the GoDAM media tab (job_type=pdf,document), the Elementor, WPBakery and shortcode pickers, and the Retranscode Media row action. - Keep the viewer CSS in the block style rather than the lazy chunk, so it reaches the editor canvas iframe; a runtime-injected stylesheet lands in the outer admin document and never arrives. - Drop the client-side page-count regex; pdf.js reports numPages, which works cross-origin and handles PDF 1.5+ object streams. Only written while the block is selected, so opening a post does not dirty it. - Wait for the attachment record to resolve before deciding a document has no preview, so the block no longer flashes the unavailable panel. * fix: harden Document block previews and document classification - require the extension to agree with the MIME type, so text/plain lookalikes (.srt/.c/.h) are not offered as documents - hand pdf.js an explicit PDFWorker, so one document unmounting no longer destroys the shared worker and breaks the block - editor: try each preview URL candidate before the download-only panel, and wait for the attachment record before judging state - show transcoding status and page-0 previews for every document type in the media library, not just PDF - clear a stale preview when a document job is re-queued, and infer a missing MIME from the file name - use @SInCE n.e.x.t instead of hardcoding 2.2.0 * perf: render Document block pages on demand and make the viewer accessible - mount only the pages near the viewport, backed by placeholders of each page's real height, and release off-screen canvases: a 160-page PDF drops from 160 canvases / ~3.1 GB of canvas memory to 2-3 / ~40-60 MB, with the scroll height unchanged - keep a two-page floor and fall back to rendering everything without IntersectionObserver, since it reports nothing while a document is hidden - discard a previous document's page geometry when the file changes - label the scroll container as a focusable region, so keyboard and screen-reader users can reach and scroll it - guard view.js's native fallback against being run twice by a single failure * fix: render Document block previews on Safari below 17.4 pdf.js 4.8 calls Promise.withResolvers() unguarded in 31 places, including every document load, but the method only arrived in Safari 17.4 — so Safari 16.4-17.3 threw out of react-pdf's load effect, unmounting the viewer and leaving an empty box. Polyfill it on the main thread, and via a new worker entry on the worker thread, which shares no globals with the page. * feat: load document viewer assets in WPBakery's inline editor for immediate preview * fix: render every Document block page when the viewer starts at zero width * fix: restore the isDocumentModel import in the two-column attachment details The develop merge dropped isDocumentModel from this file's import while render() still called it, throwing a ReferenceError on every two-column attachment details render, for any attachment. * fix: sync package-lock.json with package.json The lock regenerated when react-pdf and pdfjs-dist were added had its nested duplicate entries pruned, so npm 10.9.2 (the version behind the .nvmrc Node) recomputed 44 entries the lock never recorded and npm ci failed before the unit tests could run. Rebase the lock onto develop's and re-resolve, so the only difference is the react-pdf subtree. --------- Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
* fix(analytics): show only the hotspot label in the layer rail
The player tracker writes each hotspot event's layer_name as the parent
layer name, a separator, then "Hotspot N". The detail-panel rail already
shows the parent layer as its card title, so that prefix was pure
repetition, and the row's truncation then hid the "Hotspot N" that
actually identifies it.
Strip the parent-name prefix from the sub-hotspot name so each row reads
just "Hotspot N". Woo product rows carry a bare product_name and are
left alone. This reads what is already stored, so it fixes existing data
with no change to the wire format.
Add unit tests for the strip, the untouched Woo case, and a custom name
that does not start with the parent prefix.
* fix(analytics): strip only the exact parent-name separator from hotspot labels
Match the exact "<parent> — " prefix the tracker composes instead of
stripping on the bare parent name plus a greedy run of leading dashes.
This leaves intact a custom label that merely begins with the parent
name ("Sale" -> "Sale special") and a label that itself starts with a
dash ("Summer sale — -50% off" -> "-50% off", not "50% off").
Add guard tests for both cases.
…ents (#2083) * feat(tools): let Retranscode Media fetch audio, PDF and image attachments * feat(tools): let Retranscode Media fetch every document format, not just PDF * Fixed the PR feedback --------- Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
…#2072) * feat(ci): audit rtgodam_before/after_attachment_lookup hook integrity Adds three static-analysis scripts under bin/ that guard the rtgodam_before_attachment_lookup / rtgodam_after_attachment_lookup hook pair against silent regressions, each checked against an accepted JSON baseline: - godam-wp-dam-hook-check.php: per-file hook-fire count regressions and before/after balance within a scope. - godam-attachment-access-coverage-check.php: new or growing attachment-access calls that aren't yet reviewed for hook coverage. - godam-interprocedural-leak-check.php: a self-wrapped function whose hook stays open across a call site elsewhere. Wires them into a new GitHub Actions workflow (wp_dam_hook_integrity_on_pull_request.yml) and into the local pre-commit hook via godam-hook-check-pre-commit.sh. Each script supports a `check` mode (default, used in CI) and an `update-baseline` mode (run locally after manually auditing new/changed code). * refactor: removes additional ci script * refactor: update package.json scripts * refactor: move seperate functions to a seperate godam-hook-check-shared.php file * feat: add functionality to skip ci check for perticular file path using godam_check_known_balance_exceptions * feat: update before/after attachment hook ci * feat: add bracket check to godam_shared_is_call_to * docs: Rephrase docs to reduce the amount of doc to be read in order to understand * feat: add support to catch wpdb class object call when wrapped in curly braces * refactor: migrate baseline-JSON tracking, move hook checks into bin/hook-check/ * feat: adds support to catch WP_Query class calls correctly * refactor: updates coverage files * refactor: extends hook coverage check * feat: add rtgodam_before_attachment_lookup and rtgodam_after_attachment_lookup hooks around media (#2080) * feat: add rtgodam_before_attachment_lookup and rtgodam_after_attachment_lookup hooks around media * feat: add pending rtgodam_before_attachment_lookup and rtgodam_after_attachment_lookup hooks around media * fix: fixes core godam bugs * refactor: update before/after attachmet CI baseline * refactor: adds godam-coverage-ignore comments * refactor: update since version * feat: adds before/after media hook * refactor: adds godam-coverage-ignore comments * refactor: adds godam-coverage-ignore comments * feat: adds before/after media hook * feat: adds before/after media hook
…2075) * fix(transcoder-admin): correct spelling of 'Trialling' to 'Trialing' * Approve both Trialing, and Trialling subscription status --------- Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
…or (#2091) Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
* Fix Document block download link for virtual media * Fix PR feedback --------- Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
…#2094) * Update the plugin version, add changelogs, and new contributors props * Generate new POT file --------- Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Assigned as-is. The URL has already been through esc_url() server-side, and encodeURI() | ||
| // would escape its percent signs a second time — turning a legitimate "%20" into "%2520" | ||
| // and pointing the object at a path that does not exist. | ||
| object.setAttribute( 'data', previewUrl ); |
Contributor
There was a problem hiding this comment.
Pull request overview
Prepares GoDAM 2.2.0 with expanded document previews, video lightboxes, media-library reliability fixes, attachment-centralization hooks, analytics updates, and CI safeguards.
Changes:
- Adds multi-format document viewing and video lightbox support.
- Improves media folders, transcription, analytics, and lazy loading.
- Adds release metadata, tests, hook-integrity checks, and workflows.
Reviewed changes
Copilot reviewed 120 out of 146 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
webpack.config.js |
Isolates media-library chunks. |
tests/stubs/i18n.php |
Adds translation test stubs. |
tests/php/RetranscodeMediaTypeMapTest.php |
Tests retranscoding MIME map. |
tests/bootstrap.php |
Expands test bootstrap. |
readme.txt |
Updates release metadata/changelog. |
README.md |
Updates version and contributors. |
phpcs.xml |
Excludes stub translation calls. |
pages/tools/retranscode-media.scss |
Styles retranscoding options. |
pages/media-library/redux/slice/folders.js |
Synchronizes folder state. |
pages/media-library/redux/api/folders.js |
Adds polling and cache tags. |
pages/media-library/data/media-grid.js |
Improves modal folder filtering. |
pages/media-library/components/search-bar/SearchBar.jsx |
Preserves selected folder metadata. |
pages/media-library/components/modal/DeleteModal.jsx |
Improves deletion handling. |
pages/media-library/components/folder-tree/FolderTree.jsx |
Replaces stale folder pages. |
pages/media-library/components/context-menu/ContextMenu.jsx |
Polls asynchronous ZIP jobs. |
pages/media-library/App.js |
Corrects root folder creation. |
pages/analytics/timeline/LayerInteractionFunnel.js |
Clamps funnel percentages. |
pages/analytics/hooks/useVideoLayerData.js |
Uses per-hotspot metrics. |
package.json |
Adds document-viewer dependencies/scripts. |
inc/templates/video-preview.php |
Adds attachment lookup hooks. |
inc/templates/video-embed.php |
Wraps transcoding lookup. |
inc/templates/godam-video-gallery.php |
Supports centralized gallery data. |
inc/classes/wpforms/class-wpforms-integration.php |
Defers WPForms initialization. |
inc/classes/wpforms/class-wpforms-field-godam-video.php |
Wraps attachment URL lookup. |
inc/classes/wpbakery-elements/class-wpb-godam-video.php |
Adds lightbox control. |
inc/classes/wpbakery-elements/class-wpb-godam-params.php |
Wraps selector previews. |
inc/classes/wpbakery-elements/class-wpb-godam-document.php |
Expands document support. |
inc/classes/sureforms/class-form-submit.php |
Documents coverage exclusion. |
inc/classes/sureforms/class-assets.php |
Documents host-post lookup. |
inc/classes/shortcodes/class-godam-document.php |
Supports additional documents. |
inc/classes/rest-api/class-virtual-media-migration.php |
Centralizes migration lookup. |
inc/classes/rest-api/class-video-sync.php |
Wraps video metadata query. |
inc/classes/rest-api/class-video-editor.php |
Wraps editor attachment queries. |
inc/classes/rest-api/class-transcription.php |
Adds centralized transcript access. |
inc/classes/rest-api/class-meta-rest-fields.php |
Exposes document preview metadata. |
inc/classes/rest-api/class-jetpack.php |
Documents non-attachment queries. |
inc/classes/rest-api/class-engagement.php |
Wraps transcoder ID lookup. |
inc/classes/rest-api/class-dynamic-shortcode.php |
Centralizes shortcode attachment reads. |
inc/classes/rest-api/class-dynamic-gallery.php |
Centralizes gallery queries. |
inc/classes/rest-api/class-ads.php |
Wraps ad attachment lookup. |
inc/classes/ninja-forms/class-ninja-forms-field-godam-recorder.php |
Documents entry-meta access. |
inc/classes/migrations/class-wpbakery-gallery-shortcode-v1-to-v2.php |
Documents host-post migration. |
inc/classes/migrations/class-godam-cpt-cleanup.php |
Documents CPT-only queries. |
inc/classes/migrations/class-gallery-v1-to-v2.php |
Documents host-post migration. |
inc/classes/migrations/class-elementor-gallery-widget-v1-to-v2.php |
Documents Elementor migration. |
inc/classes/media-library/class-media-folder-create-zip.php |
Hardens ZIP generation. |
inc/classes/lifter-lms/class-lifter-lms.php |
Documents host-post lookup. |
inc/classes/fluentforms/class-init.php |
Defers Fluent Forms initialization. |
inc/classes/elementor-widgets/class-godam-video.php |
Adds lightbox settings. |
inc/classes/elementor-widgets/class-godam-image.php |
Wraps image alt lookup. |
inc/classes/elementor-widgets/class-godam-document.php |
Expands document formats. |
inc/classes/cron-jobs/class-retranscode-failed-media.php |
Wraps retry attachment access. |
inc/classes/class-virtual-media-registrar.php |
Wraps virtual-media metadata. |
inc/classes/class-video-engagement.php |
Wraps engagement attachment reads. |
inc/classes/class-pages.php |
Gates heavy media assets. |
inc/classes/class-media-usage-backfill.php |
Documents host-post operations. |
inc/classes/class-demo-assets.php |
Wraps demo attachment access. |
inc/classes/class-assets.php |
Adds thumbnail guidance and gating. |
inc/classes/addons/class-addon-registry.php |
Defers translated notices. |
inc/classes/addons/class-abstract-addon.php |
Supports lazy dependency messages. |
godam.php |
Bumps plugin version. |
composer.json |
Updates POT generation. |
CHANGELOG.md |
Adds 2.2.0 release notes. |
bin/hook-check/pre-commit.sh |
Runs hook integrity locally. |
assets/src/js/wpbakery/wpbakery-document-selector-param.js |
Expands selectable documents. |
assets/src/js/media-library/views/godam-media-frame-shared.js |
Normalizes document filters. |
assets/src/js/media-library/views/attachment.js |
Shows document statuses. |
assets/src/js/media-library/views/attachment-detail-two-column.js |
Adds lazy Video.js loading. |
assets/src/js/media-library/views/attachment-browser.js |
Adjusts browser overrides. |
assets/src/js/media-library/videojs-loader.js |
Adds Video.js lazy loader. |
assets/src/js/media-library/utility.js |
Adds document/view helpers. |
assets/src/js/godam-player/utils/storage.test.js |
Tests batched interaction storage. |
assets/src/js/godam-player/utils/storage.js |
Batches interaction persistence. |
assets/src/js/godam-player/utils/seekPlayer.js |
Adds metadata-safe seeking. |
assets/src/js/godam-player/utils/customFullscreen.js |
Extracts fullscreen helpers. |
assets/src/js/godam-player/managers/transcriptPanelManager.js |
Prefers stored transcripts. |
assets/src/js/godam-player/managers/transcriptManager.js |
Honors stored/deleted transcripts. |
assets/src/js/godam-player/managers/shareManager.js |
Uses attachment page anchors. |
assets/src/js/godam-player/managers/README.md |
Documents lightbox architecture. |
assets/src/js/godam-player/managers/playerManager.js |
Registers lightbox players. |
assets/src/js/godam-player/managers/controlsManager.js |
Reuses fullscreen utilities. |
assets/src/js/godam-player/lightboxTriggers.js |
Adds delegated lightbox triggers. |
assets/src/js/godam-player/api/godam-api.js |
Exposes lightbox API. |
assets/src/js/godam-player/analytics.js |
Chunks hotspot analytics. |
assets/src/js/godam-image-layers/render-image-frame.js |
Observes asynchronous icons. |
assets/src/js/elementor/editor.js |
Locks incompatible controls. |
assets/src/js/elementor/controls/godam-media.js |
Supports multiple MIME types. |
assets/src/css/media-library.scss |
Styles thumbnail guidance. |
assets/src/css/admin.scss |
Updates Elementor lock styles. |
assets/src/blocks/godam-video-thumbnail/render.php |
Wraps thumbnail metadata. |
assets/src/blocks/godam-video-duration/render.php |
Wraps duration metadata. |
assets/src/blocks/godam-player/render.php |
Documents host-post metadata. |
assets/src/blocks/godam-player/editor.scss |
Styles thumbnail guidance. |
assets/src/blocks/godam-player/edit.js |
Adds lightbox editor behavior. |
assets/src/blocks/godam-player/edit-common-settings.js |
Adds lightbox toggle. |
assets/src/blocks/godam-player/components/ThumbnailPanel.js |
Renders filtered guidance. |
assets/src/blocks/godam-player/block.json |
Declares lightbox attribute. |
assets/src/blocks/godam-pdf/viewer/worker.js |
Configures shared PDF worker. |
assets/src/blocks/godam-pdf/viewer/viewer.scss |
Styles PDF viewer. |
assets/src/blocks/godam-pdf/viewer/promise-with-resolvers.js |
Polyfills older browsers. |
assets/src/blocks/godam-pdf/viewer/pdf-worker.js |
Adds worker entrypoint. |
assets/src/blocks/godam-pdf/style.scss |
Adds viewer/fallback styles. |
assets/src/blocks/godam-pdf/preview-error-boundary.js |
Handles viewer failures. |
assets/src/blocks/godam-pdf/block.json |
Documents supported formats. |
assets/src/blocks/godam-image/render.php |
Wraps image attachment reads. |
assets/src/blocks/godam-gallery-v2/style.scss |
Improves modal sizing. |
admin/godam-transcoder-actions.php |
Expands document handling. |
admin/class-rtgodam-transcoder-admin.php |
Accepts trial status spellings. |
.husky/pre-commit |
Adds hook-integrity check. |
.github/workflows/wp_dam_hook_integrity_on_pull_request.yml |
Adds hook-integrity CI. |
.github/workflows/pot_up_to_date_on_pull_request.yml |
Verifies translation template. |
.github/workflows/php_unit_tests_on_pull_request.yml |
Adds PHPUnit CI. |
.github/workflows/js_unit_tests_on_pull_request.yml |
Uses default PR checkout. |
.distignore |
Excludes PHP tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+64
to
+69
| // The bulk endpoint returns HTTP 200 with `errors` on *partial* failure, so | ||
| // .unwrap() alone won't throw. On a partial failure we still remove the whole | ||
| // selection optimistically and surface a WARNING (not "deleted successfully"); | ||
| // the invalidation refetch below re-adds any folder that wasn't actually | ||
| // deleted, so successfully-deleted folders don't linger (even on load-more | ||
| // pages, where the reducer removes them from the flat list). |
Comment on lines
+26
to
+29
| loadingPromise = import( /* webpackChunkName: "videojs" */ 'video.js' ).then( ( mod ) => { | ||
| cachedVideojs = mod.default || mod; | ||
| return cachedVideojs; | ||
| } ); |
Comment on lines
+45
to
+46
| } catch ( error ) { | ||
| global.console?.warn( 'GoDAM: could not start the pdf.js worker; rendering on the main thread instead', error ); |
| true, | ||
| ); | ||
|
|
||
| prepareTriggers(); |
Comment on lines
497
to
+502
| fetch( endpoint + '/analytics/', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify( body ), | ||
| keepalive: true, | ||
| } ); | ||
| } ).catch( () => {} ); |
Comment on lines
+155
to
159
| do_action( 'rtgodam_before_attachment_lookup' ); | ||
|
|
||
| // Locally-stored transcript takes precedence. | ||
| $path = get_post_meta( $attachment_id, 'rtgodam_transcript_path', true ); | ||
|
|
🔍 WordPress Plugin Check Report
📊 Report
❌ Errors (1)📁 readme.txt (1 error)
|
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
mismatched_plugin_name | Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM". |
0 |
trademarked_term | The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name. |
📁 composer.json (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
missing_composer_json_file | The "/vendor" directory using composer exists, but "composer.json" file is missing. |
📁 assets/build/blocks/godam-gallery-v2/render.php (2 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
15 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound | Global variables defined by a theme/plugin should start with the theme/plugin prefix. Found: "$inner_block_video_ids". |
23 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound | Global variables defined by a theme/plugin should start with the theme/plugin prefix. Found: "$inner_block_video_ids". |
📁 assets/build/css/main.css (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
EnqueuedStylesScope | This style is being loaded in all contexts. |
📁 assets/src/libs/analytics.min.js (5 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
EnqueuedScriptsScope | This script is being loaded in all frontend contexts. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/2026/08/26/hello-world/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/sample-page/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead. |
📁 assets/build/js/main.min.js (5 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
EnqueuedScriptsScope | This script is being loaded in all frontend contexts. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/2026/08/26/hello-world/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/sample-page/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
0 |
NonBlockingScripts.NoStrategy | This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead. |
🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 146 changed files in this pull request and generated 2 comments.
Suppressed comments (3)
assets/src/js/media-library/videojs-loader.js:29
- A failed dynamic import is cached forever in
loadingPromise. The caller deliberately catches transient chunk/network failures, but every later attachment-detail open will receive the same rejected promise, so Video.js cannot recover until the page reloads. Clear the cached promise on rejection so a later open retries.
assets/src/js/godam-player/lightboxTriggers.js:146 - This one-time pass does not give dynamically injected non-interactive triggers the promised keyboard semantics. The delegated click handler will work for later AJAX/page-builder markup, but an injected
<div>or<img>never receivesrole="button"ortabindex="0", so keyboard users cannot reach it. Observe added triggers (or require callers to invoke preparation) as well as delegating activation.
assets/src/js/godam-player/analytics.js:477 - The 32 KiB limit is per chunk, but the Fetch keepalive quota described above is aggregate across all in-flight requests. This loop launches every chunk without waiting, alongside the type-1/type-2 keepalive requests, so two or more chunks can still exceed the ~64 KiB page budget; later requests reject and the unconditional buffer clear loses those interactions. The flush needs a total-budget strategy or an earlier non-unload transport, not only per-request chunking.
Comment on lines
+152
to
+153
| openLightbox( attachmentID, options = {} ) { | ||
| return openLightboxForId( attachmentID, options ); |
Comment on lines
33
to
43
| bindEvents() { | ||
| // Chain to the parent's bindEvents (if any) so other code extending | ||
| // the same wp.media.view.AttachmentsBrowser (e.g. wp-dam) isn't | ||
| // silently dropped by this override. | ||
| if ( AttachmentsBrowser.prototype.bindEvents ) { | ||
| AttachmentsBrowser.prototype.bindEvents.apply( this, arguments ); | ||
| } | ||
|
|
||
| this.collection.props.on( 'change', this.updateCollectionObserve, this ); | ||
| this.collection.props.on( 'change', this.addUploadParam, this ); | ||
| }, |
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.
No description provided.