- Issue created by @sokru
- last update
about 1 year ago 46 pass - Status changed to Needs review
about 1 year ago 10:46am 6 June 2024 - Assigned to Grevil
- 🇩🇪Germany Anybody Porta Westfalica
Thanks @sokru - we just made a change to the file, could you fix the conflicts please?
@Grevil could you check this afterwards please to be committed?
- last update
about 1 year ago 46 pass - First commit to issue fork.
- Merge request !37Issue #3452930 by herved, grevil: Add config schema for field.formatter settings and remove "use_form" key → (Merged) created by herved
- 🇧🇪Belgium herved
I noticed a few issues and opened MR to address those https://git.drupalcode.org/project/spamspan/-/merge_requests/37
- At the moment the formatter settings with layout builder are not saved correctly and values under
use_form
are nested and not flattened. This happens because the form structure with layout builder is different than with field display overview form so the code inEmailSpamspanFormatter::validateSettingsForm
is not correct.
- To solve this I removed this validate handler and use a #pre_render to display the use_form details while keeping the values flat.
This way it works on both and we do not need to know the entire form structure or make conditions.
- Added a test for the spamspan formatter and config schema tests for that and filter format
- Also centralized config schema under spamspan_plugin_settings so we can avoid repetition use it for the formatter and filter. - 🇩🇪Germany Grevil
Thank you, @herved! This looks great!
Unsure, what to do with the Drupal requirement. Mentioned @joevagyok for this, what he thinks about dropping the D9 / D10.0.0 support in a minor / patch release.
- 🇧🇪Belgium joevagyok
Dropping D9 in a minor release, is fine. We should maintain Drupal releases up until their EOL. Based on the discussion in the MR, I would suggest adapting the core version constraints to
^10.1 || ^11
and making sure we have these respective builds in gitlab CI, which we do, by looking into the green pipeline. - 🇩🇪Germany Grevil
LGTM! Once the tests are green, we can merge this! :)
Thanks for your contribution @herved!
Automatically closed - issue fixed for 2 weeks with no activity.