Added files #1
Open
Added files #1
Conversation
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
…into release/first_version
rimi-itk
requested changes
Oct 9, 2025
| # itk_video | ||
| # ITK Video | ||
|
|
||
| Module that supplies video integration, through a dedicated itk_video field |
There was a problem hiding this comment.
Suggested change
| Module that supplies video integration, through a dedicated itk_video field | |
| Module that supplies video integration through a dedicated itk_video field |
Comment on lines
+13
to
+18
| The definitions include; | ||
|
|
||
| - host : Used to identify the provider from the supplied video URL. | ||
| - type : Used to determine if videoinformation should be retrieved bu this | ||
| module (custom) or Oembed | ||
| - requiredCookies : The cookies that the provider sets when embedding a video |
There was a problem hiding this comment.
Suggested change
| The definitions include; | |
| - host : Used to identify the provider from the supplied video URL. | |
| - type : Used to determine if videoinformation should be retrieved bu this | |
| module (custom) or Oembed | |
| - requiredCookies : The cookies that the provider sets when embedding a video | |
| The definitions include | |
| - host: Used to identify the provider from the supplied video URL. | |
| - type: Used to determine if videoinformation should be retrieved bu this | |
| module (custom) or Oembed | |
| - requiredCookies: The cookies that the provider sets when embedding a video |
Comment on lines
+27
to
+28
| The module includes a permission to access the configuration page: | ||
| ```"Administer ITK Video settings"``` |
There was a problem hiding this comment.
Suggested change
| The module includes a permission to access the configuration page: | |
| ```"Administer ITK Video settings"``` | |
| The module includes a permission to access the configuration page: "Administer ITK Video settings" |
| The module includes a permission to access the configuration page: | ||
| ```"Administer ITK Video settings"``` | ||
|
|
||
| If allowed access the settings are set on ```/admin/config/media/itk-video```. |
There was a problem hiding this comment.
Suggested change
| If allowed access the settings are set on ```/admin/config/media/itk-video```. | |
| If allowed access the settings are set on `/admin/config/media/itk-video`. |
|
|
||
| ## Code | ||
|
|
||
| To check that coding standards are up to scratch run the following commands: |
| $requiredCookies = $supportedProviders[$providerKey]['requiredCookies']; | ||
|
|
||
| if (!empty($requiredCookies) && isset($videoArray['iframe'])) { | ||
| $videoArray['iframe'] = str_replace(' src="', ' src="" data-category-consent="' . $requiredCookies . '" data-consent-src="', $videoArray['iframe']); |
There was a problem hiding this comment.
This probably works, but it makes a lot of assumptions on the structure of the iframe HTML.
Comment on lines
+200
to
+201
| $blockedText .= '<br>'; | ||
| $blockedText .= $fieldValue->title ? '"' . $fieldValue->title . '"' : ''; |
There was a problem hiding this comment.
Suggested change
| $blockedText .= '<br>'; | |
| $blockedText .= $fieldValue->title ? '"' . $fieldValue->title . '"' : ''; | |
| if ($fieldValue->title) { | |
| $blockedText .= '<br>"' . $fieldValue->title.'"'; | |
| } |
Comment on lines
+28
to
+29
| 'title' => DRUPAL_REQUIRED, | ||
| 'link_type' => LinkItemInterface::LINK_EXTERNAL, |
There was a problem hiding this comment.
I don't understand how this “title” trick is used …
| // However, if it is inaccessible to the current user, do not display it | ||
| // to them. | ||
| // @phpstan-ignore globalDrupalDependencyInjection.useDependencyInjection | ||
| if (\Drupal::currentUser()->hasPermission('link to any page') || $item->getUrl()->access()) { |
There was a problem hiding this comment.
Use dependency injection to get the current user.
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function formElement(FieldItemListInterface $items, $delta, array $element, array &$form, FormStateInterface $form_state) { |
There was a problem hiding this comment.
I don't understand what's going on in this function, but it seems complicated.
I suppose that inserting a video requires both a title and a URL. Do we need this much code to ensure that?
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.
Includes