Task #771 - Embed video block added #830

DjidjaleskaSandra · 2024-06-13T10:36:36Z

Description

Use the Embed Video Block to seamlessly embed videos. Paste the video’s embed URL (e.g., https://www.youtube.com/embed/dQwsxXcQ), and the block will display the video directly on your site.

Screenshots / Videos

Task

https://app.productive.io/1-infinum/time/me/task/8267996?tour=day-view-layout

goranalkovic-infinum

I'm not very happy with this, looks like it was just copied over without any adaptation to the rest of the current FE libs components.

Please resolve all the comments, and:

use components from FE libs so the UI/UX matches the rest of components
fix all textdomains, they can't be safer-internet, should be '%g_textdomain%'

goranalkovic-infinum · 2024-06-13T10:39:47Z

blocks/init/src/Blocks/custom/embed/manifest.json

+		}
+	},
+	"variables": {
+		"embedAlign": {


The align option is not needed

goranalkovic-infinum · 2024-06-13T10:40:44Z

blocks/init/src/Blocks/components/embed/components/embed-editor.js

+	const embedClass = classnames([
+		selector(componentClass, componentClass),
+		selector(blockClass, blockClass, selectorClass),
+		selector(additionalClass, additionalClass),
+	]);


Suggested change

const embedClass = classnames([

selector(componentClass, componentClass),

selector(blockClass, blockClass, selectorClass),

selector(additionalClass, additionalClass),

]);

const embedClass = classnames(

componentClass,

bem(blockClass, selectorClass),

additionalClass

);

Import classnames and bem from Frontend libs

goranalkovic-infinum · 2024-06-13T10:41:07Z

blocks/init/src/Blocks/components/embed/components/embed-editor.js

+	const embedIframeClass = classnames([
+		selector(componentClass, componentClass, 'iframe'),
+		selector(blockClass, blockClass, `${selectorClass}-iframe`),
+	]);


Suggested change

const embedIframeClass = classnames([

selector(componentClass, componentClass, 'iframe'),

selector(blockClass, blockClass, `${selectorClass}-iframe`),

]);

const embedIframeClass = classnames(

selector(componentClass, 'iframe'),

selector(blockClass, `${selectorClass}-iframe`),

);

goranalkovic-infinum · 2024-06-13T10:41:45Z

blocks/init/src/Blocks/components/embed/components/embed-editor.js

+	if (embedUrl.includes('https://www.youtube.com/watch?v=')) {
+		embedUrl = embedUrl.replace('watch?v=', 'embed/');
+	}
+
+	if (embedUrl.includes('https://vimeo.com')) {
+		const matches = [...embedUrl.matchAll(/https:\/\/vimeo\.com\/(\d+)/g)];
+
+		const videoId = matches[0][1] ?? '';
+		embedUrl = `https://player.vimeo.com/video/${videoId}`;
+	}


This should be removed so any embed link can work, not only YouTube and Vimeo.

goranalkovic-infinum · 2024-06-13T10:42:00Z

blocks/init/src/Blocks/components/embed/components/embed-editor.js

+	return (
+		<>
+			{embedUse &&
+				<>
+					{(embedUrl === '') ?
+						<Placeholder icon={video} label={__('Please add embed using sidebar options!', 'safer-internet')} /> :
+						<div className={embedClass}>
+							<iframe
+								title={'video'}
+								className={embedIframeClass}
+								src={embedUrl}
+							/>
+						</div>
+					}
+				</>
+			}
+		</>
+	);


This code is pretty messy, please clean it up

goranalkovic-infinum · 2024-06-13T10:46:53Z

blocks/init/src/Blocks/components/embed/embed-style.scss

+.embed {
+	position: relative;
+	width: 100%;
+	overflow: hidden;
+	padding-top: 56.25%;
+
+	&__iframe {
+		position: absolute;
+		top: 0;
+		left: 0;
+		width: 100%;
+		height: 100%;
+		border: 0;
+	}
+}


Please rework this so it doesn't use the old aspect ratio hack, we've had aspect-ratio in all browsers for 3 years now...

goranalkovic-infinum · 2024-06-13T10:47:38Z

blocks/init/src/Blocks/components/embed/embed.php

+$embedClass = Helpers::classnames([
+	Helpers::selector($componentClass, $componentClass),
+	Helpers::selector($blockClass, $blockClass, $selectorClass),
+	Helpers::selector($additionalClass, $additionalClass),
+]);
+
+$embedIframeClass = Helpers::classnames([
+	Helpers::selector($componentClass, $componentClass, "iframe"),
+	Helpers::selector($blockClass, $blockClass, "{$selectorClass}-iframe"),
+]);


Suggested change

$embedClass = Helpers::classnames([

Helpers::selector($componentClass, $componentClass),

Helpers::selector($blockClass, $blockClass, $selectorClass),

Helpers::selector($additionalClass, $additionalClass),

]);

$embedIframeClass = Helpers::classnames([

Helpers::selector($componentClass, $componentClass, "iframe"),

Helpers::selector($blockClass, $blockClass, "{$selectorClass}-iframe"),

]);

$embedClass = Helpers::classnames([

$componentClass,

Helpers::selector($blockClass, $blockClass, $selectorClass),

$additionalClass,

]);

$embedIframeClass = Helpers::classnames([

Helpers::selector($componentClass, $componentClass, "iframe"),

Helpers::selector($blockClass, $blockClass, "{$selectorClass}-iframe"),

]);

goranalkovic-infinum · 2024-06-13T10:48:22Z

blocks/init/src/Blocks/components/embed/embed.php

+/**
+ * YouTube provider
+ *
+ * The link
+ *   https://www.youtube.com/watch?v=dQw4w9WgXcQ
+ * will be transformed to
+ *   https://www.youtube.com/embed/dQw4w9WgXcQ .
+ */
+if (strpos($embedUrl, 'https://www.youtube.com/watch?v=') !== false) {
+	$embedUrl = str_replace('watch?v=', 'embed/', $embedUrl);
+}
+
+/**
+ * Vimeo provider
+ *
+ * The link
+ *   https://vimeo.com/642263700
+ * will be transformed to
+ *   https://player.vimeo.com/video/642263700 .
+ */
+if (strpos($embedUrl, 'https://vimeo.com') !== false) {
+	preg_match_all('/https:\/\/vimeo\.com\/(\d+)/', $embedUrl, $matches);
+
+	$videoId = $matches[1][0] ?? '';
+	$embedUrl = "https://player.vimeo.com/video/{$videoId}";
+}


Please replace this so it can work with any site.

Note
For videos, WordPress does have a built-in function for this, but it's very bad for performance. Find a solution to store its output if you're planning to use it.

However, we should probably not support only videos.

goranalkovic-infinum · 2024-06-13T10:48:32Z

blocks/init/src/Blocks/components/embed/embed.php

+
+?>
+
+<div class="<?php echo esc_attr($embedClass); ?>">


Does this need a wrapping div?

The div is used for video position

goranalkovic-infinum · 2024-06-13T10:48:48Z

blocks/init/src/Blocks/custom/embed/components/embed-toolbar.js

+	const embedAlign = checkAttr('embedAlign', attributes, manifest);
+
+	return (
+		<MatrixAlignControl


Don't need this

DjidjaleskaSandra · 2024-06-13T11:44:21Z

I'm not very happy with this, looks like it was just copied over without any adaptation to the rest of the current FE libs components.

Please resolve all the comments, and:

use components from FE libs so the UI/UX matches the rest of components

fix all textdomains, they can't be safer-internet, should be '%g_textdomain%'

@goranalkovic-infinum Thank you for the suggestions. The PR is not yet ready for review. I will inform you when it is.

iruzevic · 2024-06-24T07:26:38Z

blocks/init/src/Blocks/custom/embed/components/embed-options.js

+
+export const EmbedOptions = ({ attributes, setAttributes }) => {
+	return (
+		<PanelBody title={__('Embed', 'safer-internet')}>


wrong text domain

iruzevic · 2024-06-24T07:28:54Z

blocks/init/src/Blocks/custom/embed/embed-style.scss

+}
+
+/** Add style to WP core embed block **/
+.wp-block-embed__wrapper {


what is this used for?

iruzevic · 2024-06-24T07:29:04Z

blocks/init/src/Blocks/custom/embed/embed.php

+/**
+ * Template for the Embed Block view.
+ *
+ * @package SaferInternet


wrong text domain

DjidjaleskaSandra added 4 commits June 13, 2024 11:14

Embed video block added

75e53c8

remove depricatedmethod

3164c99

missed shema key added

a87d851

missed shema key added

967d6c2

DjidjaleskaSandra added the feature request New feature request label Jun 13, 2024

DjidjaleskaSandra self-assigned this Jun 13, 2024

goranalkovic-infinum requested changes Jun 13, 2024

View reviewed changes

DjidjaleskaSandra added 2 commits June 13, 2024 13:02

remove depricated method

914d69c

remove unneccesery global manifest

b38a738

DjidjaleskaSandra marked this pull request as draft June 13, 2024 11:44

DjidjaleskaSandra added 5 commits June 13, 2024 16:50

update component according latest eightshift standards

abcd57d

update component according latest eightshift standards

dc87ed7

adjust video permitions

b0c2c20

simplify helpers::clasnames attrs

d2b770b

added clear instruction about URL format

e7ec03b

DjidjaleskaSandra requested a review from goranalkovic-infinum June 13, 2024 21:14

DjidjaleskaSandra marked this pull request as ready for review June 13, 2024 21:14

DjidjaleskaSandra requested a review from iruzevic June 13, 2024 21:15

iruzevic requested changes Jun 24, 2024

View reviewed changes

iruzevic closed this Nov 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Task #771 - Embed video block added #830

Task #771 - Embed video block added #830

DjidjaleskaSandra commented Jun 13, 2024 •

edited

Loading

goranalkovic-infinum left a comment •

edited

Loading

goranalkovic-infinum Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

DjidjaleskaSandra Jun 13, 2024

goranalkovic-infinum Jun 13, 2024

DjidjaleskaSandra commented Jun 13, 2024

iruzevic Jun 24, 2024

iruzevic Jun 24, 2024

iruzevic Jun 24, 2024

Task #771 - Embed video block added #830

Task #771 - Embed video block added #830

Conversation

DjidjaleskaSandra commented Jun 13, 2024 • edited Loading

Description

Screenshots / Videos

Task

goranalkovic-infinum left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DjidjaleskaSandra commented Jun 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DjidjaleskaSandra commented Jun 13, 2024 •

edited

Loading

goranalkovic-infinum left a comment •

edited

Loading