Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Vimeo video embeds with cookie consent handling, similar to the existing YouTube handler. The implementation adds a new Vimeo handler class that intercepts Vimeo embed HTML and wraps it with consent placeholders, requiring marketing consent before displaying the video.
- Added a new
Vimeohandler class with placeholder functionality for cookie consent - Registered the Vimeo handler in the plugin initialization
- Updated changelog to document the new feature
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Handlers/Vimeo.php | New handler class that adds consent placeholders to Vimeo embeds |
| src/CookiebotPlugin.php | Registered the new Vimeo handler in the handlers array |
| CHANGELOG.md | Added entry documenting Vimeo support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return $html; | ||
| } | ||
|
|
||
| $placeholder = Markup::get_wrapper_start( $this->opt_out_type ); |
There was a problem hiding this comment.
The placeholder variable is initialized with a wrapper start but then immediately overwritten by the switch statement logic. This initial assignment on line 102 serves no purpose and should be removed.
| $placeholder = Markup::get_wrapper_start( $this->opt_out_type ); |
| * @param string $html The placeholder HTML string. | ||
| */ | ||
| $placeholder = \apply_filters( 'cookiebot_helper_vimeo_placeholder_output', $placeholder ); | ||
| $placeholder = Markup::get_wrapper_start( $this->opt_out_type ) . $placeholder . Markup::get_wrapper_end(); |
There was a problem hiding this comment.
The wrapper is being added twice: once on line 102 and again on line 129. This will result in duplicate wrapper elements around the placeholder. Remove the wrapper assignment on line 102 to fix this duplication.
1 participant
Add vimeo embed support