Complete modernization: 7-class architecture, PHPStan Level 9, perfor… by skerbis · Pull Request #52 · FriendsOfREDAXO/vidstack

skerbis · 2025-10-19T19:30:38Z

…mance optimizations

Renamed package from 'video' to 'vidstack_player' (1.0.0-beta1)
Separated monolithic class into 7 focused classes:
- VidstackPlayer: Fluent API for player generation
- PlatformDetector: YouTube/Vimeo/Local detection
- AssetHelper: DRY asset loading with lazy initialization
- Utilities: HTML attributes, MIME detection
- Translator: i18n helper
- OembedParser: CKE5 integration
- BackendIntegration: Mediapool sidebar, FFmpeg
Code quality: PHPStan Level 9 (0 errors), Psalm (0 errors, 95.54%), ESLint (0 errors)
Performance: Lazy asset loading, array+implode optimization, URL pre-generation
GitHub workflows: code-style, build-assets, publish-to-redaxo
Complete PHPDoc coverage with generics
Modern build system with ESLint + esbuild

…mance optimizations - Renamed package from 'video' to 'vidstack_player' (1.0.0-beta1) - Separated monolithic class into 7 focused classes: * VidstackPlayer: Fluent API for player generation * PlatformDetector: YouTube/Vimeo/Local detection * AssetHelper: DRY asset loading with lazy initialization * Utilities: HTML attributes, MIME detection * Translator: i18n helper * OembedParser: CKE5 integration * BackendIntegration: Mediapool sidebar, FFmpeg - Code quality: PHPStan Level 9 (0 errors), Psalm (0 errors, 95.54%), ESLint (0 errors) - Performance: Lazy asset loading, array+implode optimization, URL pre-generation - GitHub workflows: code-style, build-assets, publish-to-redaxo - Complete PHPDoc coverage with generics - Modern build system with ESLint + esbuild

Copilot

Pull Request Overview

This PR represents a complete modernization and restructuring of the Vidstack video player addon for REDAXO. The main purpose is to transform the monolithic architecture into a modular, maintainable system with modern PHP practices and enhanced functionality.

Key changes include:

Package rename from 'vidstack' to 'vidstack_player' with version reset to 1.0.0-beta1
Architectural refactor: Split monolithic Video class into 7 focused classes (VidstackPlayer, PlatformDetector, AssetHelper, Utilities, Translator, OembedParser, BackendIntegration)
Modern fluent API implementation with method chaining for improved developer experience

Reviewed Changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
package.yml	Package rename and version reset to 1.0.0-beta1
lib/video.php	Removed monolithic 715-line Video class
lib/VidstackPlayer.php	New main player class with fluent API (388 lines)
lib/PlatformDetector.php	Extracted platform detection logic (97 lines)
lib/AssetHelper.php	DRY asset loading with cache-busting (110 lines)
lib/Utilities.php	HTML attributes and MIME type utilities (71 lines)
lib/Translator.php	i18n translation helper (45 lines)
lib/OembedParser.php	CKE5 integration for oEmbed parsing (48 lines)
lib/BackendIntegration.php	Mediapool sidebar and FFmpeg integration (150 lines)
lang/translations.php	Simplified translation structure
boot.php	Updated to use new class structure with lazy loading
assets/	Updated helper JavaScript and CSS files
README.md	Complete rewrite with modern documentation
composer.json	New Composer configuration
build/	Updated build system with ESLint
tests/README.md	Test structure documentation (coming soon)

Files not reviewed (1)

build/package-lock.json: Language not supported

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-19T19:31:32Z

+ } else {
+ /**
+ * @psalm-suppress MixedOperand
+ * @phpstan-ignore binaryOp.invalid


The PHPStan ignore comment uses an invalid rule name. The correct rule name should be 'binaryOp.invalidTypes' instead of 'binaryOp.invalid'.
Suggested change
* @phpstan-ignore binaryOp.invalid
* @phpstan-ignore binaryOp.invalidTypes

Copilot · 2025-10-19T19:31:33Z

+ }
+
+ try {
+ /** @phpstan-ignore class.notFound */


The PHPStan ignore comment uses an invalid rule name. The correct rule name should be 'class.notFound' -> 'class.notFoundInClassPath' or similar. Consider checking PHPStan documentation for the exact rule name.
Suggested change
/** @phpstan-ignore class.notFound */
/** @phpstan-ignore class.notFoundInClassPath */

Copilot · 2025-10-19T19:31:33Z

+ */
 async function loadTranslations() {
 try {
 translations = await (await fetch('/assets/addons/vidstack/translations.json')).json();


The fetch URL still references the old addon name 'vidstack' but should reference 'vidstack_player' to match the renamed package.
Suggested change
translations=await(awaitfetch('/assets/addons/vidstack/translations.json')).json();
translations=await(awaitfetch('/assets/addons/vidstack_player/translations.json')).json();

Copilot · 2025-10-19T19:31:33Z

+ * @param string $key Attribute name
+ * @param mixed $value Attribute value
+ */
+ public function attr(string $key, $value): self


The parameter $value should have a type declaration. Consider using mixed for PHP 8.0+ or string|bool|int|null for more specific typing.
Suggested change
publicfunction attr(string$key, $value): self
publicfunction attr(string$key, mixed$value): self

coderabbitai · 2025-10-19T19:31:47Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch renovatio

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Export-ignore development files: - /build (source files) - /tests - /.github (workflows) - /.tools (phpstan bootstrap) - All config files (composer.json, .php-cs-fixer, etc.)

- Removed automatic OembedParser::register() from boot.php - Users must manually activate in project addon if needed - Prevents conflicts with custom consent solutions - Updated README.md and README_de.md with activation instructions

- Addon is just a player, not a consent solution - Focus on opt-in approach and preventing conflicts

- Fix PHPStan rule names: binaryOp.invalid → binaryOp.invalidTypes - Fix PHPStan rule names: class.notFound → undefinedClass - Fix assets path: /assets/addons/vidstack → vidstack_player - Add mixed type declaration to attr() method for PHP 8+ - Fix variable naming inconsistency: $info → $videoInfo

Copilot

Pull Request Overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

build/package-lock.json: Language not supported

Comments suppressed due to low confidence (1)

lang/translations.php:1

Package name migration note shows identical names. Should be vidstack → vidstack_player to reflect the actual package rename.

<?php

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

- Prefix all JS functions: vidstackLoadTranslations(), vidstackApplyTranslations(), etc. - Prefix all custom CSS classes: .vidstack-video-container, .vidstack-video-wrapper - Remove unused A11Y CSS (no longer in HTML output) - Prevents naming conflicts with other scripts/styles

- All assets now minified: .min.js and .min.css - Build process: esbuild with --minify flag - Removed unused files: vs_js_out.js, vs_js_out.css - Source files (vidstack_helper.js/css) now in .gitignore - Only production-ready minified files in assets/ - Reduced file sizes: ~50% smaller Assets now: - vidstack.min.js (355.5kb, was 767.7kb) - vidstack.min.css (69.7kb, was 78.5kb) - vidstack_helper.min.js (1.1kb) - vidstack_helper.min.css (2.0kb) - translations.json

Source files now organized: - assets/src/vidstack_helper.js (source) - assets/src/vidstack_helper.css (source) - assets/vidstack_helper.min.js (compiled) - assets/vidstack_helper.min.css (compiled) Benefits: - Source and compiled files in same directory tree - Clear separation: src/ vs production files - Easier to find and edit source files

Frontend (with translations + helpers): - vidstack.min.js (357K) - Vidstack Core + Helper + Translations - vidstack.min.css (72K) - Vidstack Core + Helper Styles Backend (mediapool preview only): - vidstack-backend.min.js (356K) - Vidstack Core only - vidstack-backend.min.css (70K) - Vidstack Core only Benefits: - Backend loads faster (no translation logic) - Frontend gets everything in 2 files (1 JS + 1 CSS) - Clear separation of concerns - Smaller backend payload

Update asset checks and uploads: - vidstack.js → vidstack.min.js - vidstack.css → vidstack.min.css - Add vidstack-backend.min.js - Add vidstack-backend.min.css

Workflow improvements: - Add 'renovatio' branch to triggers - Update actions/checkout v3 → v4 - Update Node.js 20 → 22 - Split ESLint into two steps (config + source files) - Lint both build/config/*.js and assets/src/*.js ESLint config improvements: - Add assets/src/**/*.js linting - Proper globals for browser environment - sourceType: 'script' for IIFE (not module) - Cover jQuery, MutationObserver, etc. All ESLint checks pass locally ✓

Features: - Manual trigger via GitHub Actions UI - Optional checkbox to commit and push built assets - Bot commits with [skip ci] to prevent loops - Useful for updating assets after dependency updates Usage: 1. Go to Actions tab 2. Select 'Build Assets & ESLint' 3. Click 'Run workflow' 4. Check 'Commit and push built assets' if needed 5. Click 'Run workflow' button

Fix: Fatal error when FFmpeg addon is installed but class not loaded - Check if FFmpeg addon is available (isAvailable) - Check if VideoInfo class exists (class_exists) - Prevents fatal error if addon is in wrong state - Gracefully returns null if FFmpeg not ready Error was: Class FriendsOfRedaxo\FFmpeg\VideoInfo not found

CopilotAI review requested due to automatic review settings October 19, 2025 19:30

skerbis marked this pull request as draft October 19, 2025 19:31

CopilotAI reviewed Oct 19, 2025
View reviewed changes

skerbis added 5 commits October 19, 2025 21:33

Add .gitattributes for clean release ZIPs
1629913
Export-ignore development files: - /build (source files) - /tests - /.github (workflows) - /.tools (phpstan bootstrap) - All config files (composer.json, .php-cs-fixer, etc.)

Add German README (README_de.md)
7460af7

Remove misleading consent management reference from CKE5 docs
d0085a4
- Addon is just a player, not a consent solution - Focus on opt-in approach and preventing conflicts

skerbis requested a review from Copilot October 19, 2025 19:56

CopilotAI reviewed Oct 19, 2025
View reviewed changes

Comment threadlib/BackendIntegration.php Outdated
Comment threadassets/vidstack_helper.js Outdated
Comment threadbuild/package.json Outdated

skerbis added 18 commits October 19, 2025 21:58

Fix build-assets workflow for new minified asset names
ec2ec5f
Update asset checks and uploads: - vidstack.js → vidstack.min.js - vidstack.css → vidstack.min.css - Add vidstack-backend.min.js - Add vidstack-backend.min.css

Update BackendIntegration.php
087b72f

fixed assets
61a3071

merge js css
2ff3a61

Update build-assets.yml
0908a34

js fix
492b090

debug
5a66a22

debug
96939fb

icons
ce5d87b

Use working vidstack assets with icons (765KB)
c7a3dc5

Temporarily disable auto-build action until icon bundling is fixed
6dab283

skerbis added 2 commits October 20, 2025 11:37

asserts fix
055d2d8

Remove build action - using pre-built assets
44dbf81

skerbis closed this Oct 20, 2025

Navigation Menu

Uh oh!

Complete modernization: 7-class architecture, PHPStan Level 9, perfor… - #52

Complete modernization: 7-class architecture, PHPStan Level 9, perfor…#52
skerbis wants to merge 26 commits into
mainfrom
renovatio

skerbis commented Oct 19, 2025

Uh oh!

CopilotAI left a comment

Uh oh!

CopilotAIOct 19, 2025

Uh oh!

CopilotAIOct 19, 2025

Uh oh!

CopilotAIOct 19, 2025

Uh oh!

CopilotAIOct 19, 2025

Uh oh!

coderabbitaiBot commented Oct 19, 2025

Review skipped

Other AI code review bot(s) detected

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	* @phpstan-ignore binaryOp.invalid
	* @phpstan-ignore binaryOp.invalidTypes

	/** @phpstan-ignore class.notFound */
	/** @phpstan-ignore class.notFoundInClassPath */

	translations=await(awaitfetch('/assets/addons/vidstack/translations.json')).json();
	translations=await(awaitfetch('/assets/addons/vidstack_player/translations.json')).json();

	publicfunction attr(string$key, $value): self
	publicfunction attr(string$key, mixed$value): self

Uh oh!

Conversation

skerbis commented Oct 19, 2025

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

CopilotAIOct 19, 2025

Choose a reason for hiding this comment

Uh oh!

CopilotAIOct 19, 2025

Choose a reason for hiding this comment

Uh oh!

CopilotAIOct 19, 2025

Choose a reason for hiding this comment

Uh oh!

CopilotAIOct 19, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitaiBot commented Oct 19, 2025

Review skipped

Other AI code review bot(s) detected

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants