-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEATURE] Upgrade compact view to use video elements #3
base: master
Are you sure you want to change the base?
Conversation
} else { | ||
$allowedElements = GeneralUtility::trimExplode(',', $allowedElements); | ||
if (in_array('png', $allowedElements)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about other image formats? What if we only allow jpg
?
} | ||
|
||
if (in_array('mp4', $allowedElements)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, mp4
is the only allowed/available format?
@@ -1,19 +1,17 @@ | |||
<link rel="stylesheet" type="text/css" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSS isn't needed anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is handled by the JS now. I followed documentation of https://developer-docs.bynder.com/UI%20components/#compact-view where it only mentioned the <script> to setup.
<div id="bynder-compactview" | ||
data-language="{language}" | ||
data-button="Pick a file from Bynder" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you drop the button text?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give me an example, where this is shown? The inline button uses this label. I cannot see where this is used in this compact view :-)
data-autoload="true" | ||
data-fullScreen="true" | ||
data-collections="true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that work selecting collections? Or does this only allow to browse collections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, it only allowed the quick filter of collections (in the bottom)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so you still get single assets. It only enables you to browse the collections?
TODO: Refactor to retrieve asset type extensions from configuration
} else { | ||
$allowedElements = GeneralUtility::trimExplode(',', strtolower($allowedElements), true); | ||
foreach (['jpg', 'png', 'gif'] as $element) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move ['jpg', 'png', 'gif']
to the extension configuration
} | ||
|
||
if (in_array('mp4', $allowedElements)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Reimplemented the whole processing of images/videos via a Resource FileRenderer suggested in core. I want to discuss how the 'bynder' extension is inserted in the localconfiguration. I added this locally now in my site configuration but this could also be located in the ext_localconf.php; $GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext'] .= ',bynder';
$GLOBALS['TYPO3_CONF_VARS']['SYS']['mediafile_ext'] .= ',bynder'; What do you think? |
yes I think that's the way to add them https://docs.typo3.org/c/typo3/cms-core/master/en-us/Changelog/7.5/Feature-69543-IntroducedGLOBALSTYPO3_CONF_VARSSYSmediafile_ext.html#extending-this-list What do you want to do with this PR? Are you using this feature already? |
No description provided.