Cool, and yes, makes sense. Merged, thanks!
No clue then. 5.0.x is not even supported anymore in the project settings on d.o, so not idea upgrade status thinks that this is an ok version, probably a bug in there than :)
Patches always welcome!
Hmm, can't install latest release on my local machine, or on server, let's see how to get around this
committed, new release in a couple of minutes
Patch attached which should fix it, are you able to test it ?
Hmm, that's weird, but I think I know why, let me double check later!
Alright, committed, new release will be up in a couple of minutes.
Ah crap, my bad! Attached is a patch which should solve it, are you able to test it?
Should be fine now with release 8.x-3.26
Alright, merged in, new release in a couple of minutes.
alright, thanks, merged!
Ok, the dev version contained that, that was the confusing part as the current release didn't.
This works perfectly fine for 8.x-3.25. The 5.0.x branch is completely obsolete too btw.
merged!
Actually nevermind, it was moved to a class in D11
Ok, tests are happy! It looks like this might be a good approach and we don't have to warn users to update the schema. We can create a change record though explaining the new feature instead, win-win!
I'm not missing something am I ? :)
Ok tried the following:
- Used layout_plugin.settings.ds.[%parent.id] in ds.entity_display.schema.yml
- Defined layout_plugin.settings.ds.* to use ds.layout_plugin.settings.default - all layouts should pick this up normally
config inspector seems happy, let's hope the tests are too :)
So it's not possible (yet) to dynamically add new schema definitions, see ✨ Allow adding dynamic configuration schema Needs work and ConfigSchemaAlterException. Bummer, otherwise the code underneath would solve a lot.
Can't think of any other approach at the moment which doesn't involve warning users that they need to define a schema for the custom layouts in a module or theme. Let me think and experiment a bit more. But even if we don't find another solution, committing this isn't probably that much of a problem as config will still be imported afaik and everything will still keep on working.
(merged 8.x-3.x with the fork, but no other changes yet)
/**
* Implements hook_config_schema_info_alter().
*/
function ds_config_schema_info_alter(&$definitions) {
// Bummer, this is not allowed!
foreach (DS::getLayouts() as $layout_name => $layout) {
if ($layout->getClass() == 'Drupal\ds\Plugin\DsLayout') {
$definitions['layout_plugin.settings.' . $layout_name]['type'] = 'ds.layout_plugin.settings.default';
}
}
}
We had a similar issue in DS, see 🐛 Config schema issues with extended DS layouts Active to allow layouts having additional configuration. Any tips always welcome, although I don't think there is one :)
Hmm, ok , so, updated version which should do the job then I guess? How do I see the difference here, not sure how to reproduce the behavior here.
Long overdue, but finally merged. will come up in a new release soon.
Ok, I've merged parts of the branch, especially the test (which only runs locally though). Afaics, tokens are working now. If anything still is weird, please open up a new issue.
Patch makes sense. I've decided though to document this in the README (for now), as I think it's an edge case (I don't really think many people use preprocess hooks here), and (more important), I don't want to introduce another BC layer (you never know this change has impact on existing sites, although I can't think of any at the moment, but you never know)
Same as 🐛 Content tokens are not usable in prefix and suffix Needs work
Sorry for the delay, but patch looks fine, and with tests, always good!
However, I wonder, without testing myself though, whether we could change the existing logic: instead of calling replace with 'clear' => TRUE the first time, use FALSE there and then add the logic checking whether it's empty or having the same content. That way, the replace call only happens once. If, and only if, that works, we don't even need the 'hide_if_no_tokens_replaced' option at all.
Also, looking at the follow up comments in 🐛 Hide Token Field when the value data is empty Fixed - it looks like adding a trim($value) would be useful too to be really sure.
Thoughts?
See 🐛 Content tokens are not usable in prefix and suffix Needs work
Sorry for the delay, patch idea is logical, never thought about that!
Will double test what happens with custom layouts on existing projects when the schema isn't defined. I wonder if hook_config_schema_info_alter could help here (never ever used that hook, but who knows !)
It needs to be added for the DS fields as well. Since it's only in D11, we can also just do this for detection:
(int) Drupal::VERSION >= 11
Will work on this one later this day
No effect or crashes when JS is disabled, so merged, thanks!
merged, thanks!
finally!
Blocked on 📌 Remove default_argument_skip_url key from views.view.authmap Active
Yes, this blocks my tests for IndieWeb module, so RTBC :)
merged, opened another issue for the localized test
Fails locally as well, at least on D11 - will double check if I can fix it
Woopwoop
Would it also be better to get a fresh $page = $this->getSession()->getPage(); after each drupalGet call?
Seems to be fine, so leaving as is. Tests are green (apart from the current todo in localized)
committed the patch to support D11
maybe add following to gitlab file so we test (at least) the previous major?
variables:
OPT_IN_TEST_PREVIOUS_MAJOR: 1
OPT_IN_TEST_PREVIOUS_MINOR: 1
committed, but removed support for d8 and d9
Ok, used sodium_compat polyfill, works fine!
hmm, looks like this is blocked on https://www.drupal.org/project/drupalci_environments/issues/3387737#comm... →
(although sodium is bundled within PHP normally afaict from php7.2, so not sure why the package requires it in composer.json as the minimum php requirement is 7.4)
crap, testbot doesn't have sodium extension installed, checking ..
done, will push a new release in a couple of minutes.
That's not possible yet, but it's an interesting feature request though!
committed (a bit different approach though)
Ha, interesting! IIRC #submit is called when JS is not enabled, but maybe we can just simply ignore that and remove the submit handler completely. I'll try to reproduce with JS disabled first though and see if there's a crash.
Tests should be green again too, fixed that in another issue.
cf https://www.drupal.org/project/ds/issues/2888382#comment-14210609 →
Patches that create DS fields to expore more data are always welcome of course.
I'm removing the hook in 📌 Tests are failing Active
Hmm interesting, that's something new in D11.1 then I guess, since I know the tests passed in D11.0 - we can probably can get rid of that alter anyway, I'll double check during christmas holidays
Thanks! Not sure why the tests currently fail, but that's not related to this patch.
swentel → made their first commit to this issue’s fork.