Taking into account the libraries depicted in https://h5p.org/node/99838, it seems that most don't support <div>
but <iframe>
:
→
While developing custom H5P libraries for a project of ours, I also used iFrame for two reasons:
a) I wanted my solution to coexist with other H5P content in a Quiz content type I had built
b) I liked the fact that my JS/CSS code wouldn't interfere with other code of the main page
More or less, the same reasons also described in that h5p.org node you mentioned in the issue summary.
If we are to hardcode the change from the iframe to div, I think a major release should exist. I personally find it better though to instead add a setting in the formatter in order to let the user decide whether iframe or div will be used.
@catch, the only reason I discussed about the iframe -> div
here is just because I thought you had tested your fix with DIV - I'm not fully aware of what hooks/events Opigno implements, that's why. It seems this isn't the case though.
As a result, since the problems described in the issue summary are partially fixed (Explan. The resource ... was blocked due to a MIME type mismatch
), I totally agree with you on merging MR!38 and opening a follow-up issue which will be more specific for the Missing H5PIntegration settings.
error!
Setting as RTBC!
Unfortunately, it still doesn't work with Drupal 10.5 and latest H5P version (alpha6 or 2.x). All assets seems to get correctly loaded - so that's a good sign!
I still have the following errors though:
- I get an error"Uncaught ReferenceError: Drupal is not defined
" which is easily solved by adding core/drupal
as a dependency of the h5p.integration
library.
- Immediately after the fix above, I get the following error: Uncaught TypeError: can't access property "H5PIntegration", settings.h5p is undefined
for which the fix isn't straightforward.
From what I guess, it would work if the embed type of the H5P is "div" but
- Most libraries are "iframe" only
- The formatter still has iframe hardcoded
- Changing from iframe to div would require a major version release, locking websites like mine in H5p alpha3, where the issue will still be not fixed.
@catch thank you for spending time on this! Really appreciate it!
H5P is a standalone module; it may usually be used along with Opigno but it definitely isn't a requirement (as in my case), so it's important that it works fine without it.
The only extra thing to have in mind is that the decided solution should still work with aggregation either enabled or disabled.
@scott_euser I'm sure that no one ignored you :) But for a module of many downloads, it's normal that in some cases we expect two or three people to say that it's working for them in order to then set it as RTBC. For example, @randalv in #60 described a scenario where it's not working. It might be something not-related to this issue specifically but, in any case, the next one coming over should take it into account and decide whether it's ok to set it as RTBC or not - if he has the experience or the guts to do so.
I also did not get a clear answer on my question in #45 🐛 Non-translatable fields can only be changed when updating the current revision. Needs work ; but I think we can live without an answer.
So, let's keep the good news: It's RTBC! :)
I think we can close this as duplicate of 🐛 D11 Console Errors Active .
Closing as duplicate of 🐛 D11 Console Errors Active .
I add every possible issue I could find as related, just to make sure there is concentrated effort. Some already have a MR or patch, so we need to pick what's being fixed and needed in each case - or which are false positives (maybe #3200009?).
In my normal Drupal, non-Opigno, v10.5 installation, I personally resolved my issue only when I rolled back my H5P module version to v2.0.0-alpha3. I tried alpha6 and 2.x but none of the job did the job. I get some deprecation errors like Deprecated: Optional parameter $fileDir declared before required parameter $defaultLanguage is implicitly treated as a required parameter in /var/www/html/vendor/h5p/h5p-editor/h5peditor.class.php on line 376
but at least it works.
I tried comparing the changes between 2.0.0-alpha3 and 2.x but I can't say I found anything so specific other than the preprocess:false and minified: true specifications (check screenshot). But I think this should affect the jQuery library only; not the assets found in the sites/default/files
folder...
MR is now properly rebased on top of 5.x.
@ivnish or other, may we set it as RTBC? From me it's a yes of course.
I am wondering if a form/structure like the following would be a good fit + how much performance cost would it cause...
So the URL stays as it currently is but on top of that, we check from the matching sources/aliases the query arguments.
The truth is that as long as Drupal people (in general) decided that a specific side or stance should be taken and this decision was forced on the main Drupal website using a specific banner, it's easily derived that it is related to the Drupal community too.
vensires → created an issue.
Adding related issue just for reference.
According to the issuer, patch from
#3473891-11: WSOD: Call to undefined method ...::getEntity() →
solves the issue if applied to field_formatter_class
module.
Resolved conflict and rerolled.
The conflict was related to the main module not supporting Drupal version 9 anymore. So it should normally be RTBC.
Change links to match the 11.x version to avoid error messages displayed on older pages ( https://api.drupal.org/api/drupal/core%21modules%21datetime%21src%21Plug... )
Adding patch working with v8.x-4.x-beta6 versions due to changelog https://www.drupal.org/node/3458551 → .
vensires → created an issue.
Thank you both for your feedback. I am sure I wasn't clear enough but my question was actually focused on this issue's context.
💬 Support to use the datetime-local element Active was focused on the DateTime Element type as used in a render array. On the other hand, this exact issue was created in order to address . An example code came from Webform's created/completed/changed columns as displayed in https://git.drupalcode.org/project/webform/-/blob/6.3.x/src/WebformSubmissionListBuilder.php?ref_type=heads#L887.
If what @vortexcentrum says is true, I would like to find the exact commit this was fixed in. I couldn't find any recent changes to the same files the patches here proposed.
Maybe the fix happening in Drupal v11.2 is related to one of the following issues but I might be completely off:
📌
[11.x] php 8.3 follow up for enum in Datetime range formatter
Active
,
🐛
Date based template_preprocess BC code is incorrect
Active
,
💬
Support to use the datetime-local element
Active
Finding the exact commit that fixed the issue might also help generate a future-compatible patch for still supported earlier versions.
vensires → created an issue.
I also agree we could use the submodule ckeditor5_plugin_pack_font
from the "CKEditor 5 Plugin Pack" module for this functionality.
I am trying to identify what should be done and in which module...
I scanned pathauto's code and it seems the only thing actually needed is the following code:
\Drupal::service('pathauto.generator')->updateEntityAlias($entity, 'update');
So, we could directly invoke this call with a proper if pathauto module exists then
condition. The question already rose in
#2872697-23: Stop saving an entity when it gets added to or removed from a group →
by @berdir but it's not yet clear what should be done exactly since this might affect other custom or contrib modules we're not currently aware of.
I currently vote for a submodule for the pathauto implementation. It could event get installed by default as part of an upgrade if the module is enabled in the installation. This submodule could also work as a best-practice example for any other module to solve the same problem in its own context.
vensires → created an issue.
Hm... Back to Needs work then...
Thank you for your work on this. Merged!
@axioteo I checked your code and seems to be correct. We already only support Drupal versions greater than 10.2 though, so we don't actually need the compatibility layer. I have done the required change in MR. Uploading patch too.
Please validate it's working as expected and I will gladly generate a new release immediately afterwards.
vensires → made their first commit to this issue’s fork.
vensires → created an issue.
For a website of ours, I use this fulltext facet filter to search for keywords. The client asked that whether user searches for "foo bar" or "bar foo" the returned result should be the same. I tried playing around tampering the query using proper Search API Events but instead fell back on changing the original patch from this issue's MR (attached) so that the 'LIKE' operator uses multiple conditions instead using AND.
I'm sharing this here too in case we would like a different approach for this or propose a different operator... Maybe...
if ($operator === 'LIKE') {
$or_condition_group = $this->query->createConditionGroup('AND');
$tokens = preg_split('/\s+/', trim($this->fulltext->getSearch()), -1, PREG_SPLIT_NO_EMPTY);
foreach ($tokens as $token) {
$or_condition_group->addCondition($this->facet->getFieldIdentifier(), $token);
}
$this->query->addConditionGroup($or_condition_group);
}
vensires → created an issue.
vensires → created an issue.
vensires → created an issue.
vensires → created an issue.
vensires → created an issue. See original summary → .
vensires → created an issue. See original summary → .
Thank you @danrod! I see you haven't added the following line of code in the MR though. Is it intentional?
\Drupal::service('page_cache_kill_switch')->trigger();
Since you provided the patch, I set it as Needs Review for someone else to approve this and set it as RTBC.
Can you also update the MR with the changes?
Based on the previous two comments, I would take the chance to set it as RTBC but... Has anyone else experienced the I had to click on "Save configuration" twice for some reason issue described previously by @danrod?
The tricky part here is that the dl isn't a button in the editor; so no one actually knows it's there, unless you use the plaintext editor. I also checked other documentation pages like Access Policy API → and it seems you are correct. I vote +1 for this change.
PS: Might you want to ask this on slack in a broader community? Though I think it's a decent choice to change this as you proposed.
vensires → created an issue.
I closed the MR so that anyone willing to tackle this issue may start the process from the very beginning.
To be honest though, I did try to find the comment described in the issue summary but couldn't find it at all in the code. Maybe something is off here or was it just me?
vensires → made their first commit to this issue’s fork.
No, I didn't test it to be honest. It seemed like a small change. Did you review my solution yourself?
The original patch had set the return value in the catch{} block and it's actually working. My suggestion in the MR is to instead have it at the end of the function. It seems more clear to me.
@drupaldope thank you for the clarification! I set it as Needs Review then!
The my menu--navbar.html.twig file phrase in #3 confused me; I'm changing this to a support request.
Feel free to revert my change in case I was mistaken.
MR should fix the issue.
vensires → created an issue.
@wotnak, if you don't have any further issues with the MR, you might also consider mentioning either the vite plugin or just this issue in the module's project page description. I see you already have a link to the README.md file; it might just be a bit more fancy - maybe adding another "note-tip" CSS class. Whatever you decide though.
I have fixed the issue in the MR.
I also took the initiative to change the string used so that it says (minimal) and not (minimal-) if the version is missing. I think it's never actually translated so I won't introduce any further issues with this change.
I suspect it was to be set as "Reviewed & tested by the community" but the user misclicked. I set it as Needs Review for now and anyone is free to check the patch and either review it or create a MR based on it with any possible corrections.
@drupaldope, it's not clear to me whether the issue is resolved with your patch or whether the whole issue was a false report.
Could you please clarify?
So, @giannisMak do we need a new MR for this to fit what @saurav-drupal-dev described in #13?
the css should come inside toolbar file because we cannot access dropdown without the admin toolbar