- Issue created by @thomas.frobieter
@anybody: here we go: https://git.drupalcode.org/issue/drowl_layouts-3410570/-/compare/4.x...3...
All technically relevant changes are done.
- Assigned to thomas.frobieter
- 🇩🇪Germany Anybody Porta Westfalica
@thomas.frobieter: Looks good. Just one thing needs to be solved differently:
We may not change the keys in src/Plugin/Layout/DrowlLayoutsSettingsTrait.php. Can we instead use the existing keys and map their values, where they are needed?
Would have been even better if we used framework-neutral keys there in the past, but who should have known that...
2 benefits from keeping the existing keys:
- All changes are only (s)css and twig - wo we have a good chance to simple extract the twig parts per framework
- Changing these keys will be super super complicated in configIs this possible?
- Assigned to Anybody
Yeah, I thought about this. I think it's much cleaner this way, because we don't need to "translate" all these values in each single Twig file.
But .. of course, this will make things easier on the backend side.Example (one single setting):
Before:
{% if layout_settings.layout_remove_grid_gutter and layout_settings.layout_remove_grid_gutter is iterable %} {% for size in layout_settings.layout_remove_grid_gutter %} {% set row_classes = row_classes|merge(['g-' ~ size ~ '-0']) %} {% endfor %} {% elseif layout_settings.layout_remove_grid_gutter %} {% set row_classes = row_classes|merge(['g-' ~ layout_settings.layout_remove_grid_gutter ~ '-0']) %} {% endif %}
After:
{% if layout_settings.layout_remove_grid_gutter and layout_settings.layout_remove_grid_gutter is iterable %} {% for size in layout_settings.layout_remove_grid_gutter %} {% if size == 'large' %} {% set size_class = 'lg' %} {% else if size == 'medium' %} {% set size_class = 'md' %} {% else if size == 'small' %} {% set size_class = 'sm' %} {% endif %} {% set row_classes = row_classes|merge(['g-' ~ size_class ~ '-0']) %} {% endfor %} {% elseif layout_settings.layout_remove_grid_gutter %} {% if layout_settings.layout_remove_grid_gutter == 'large' %} {% set size_class = 'lg' %} {% else if layout_settings.layout_remove_grid_gutter == 'medium' %} {% set size_class = 'md' %} {% else if layout_settings.layout_remove_grid_gutter == 'small' %} {% set size_class = 'sm' %} {% endif %} {% set row_classes = row_classes|merge(['g-' ~ layout_settings.layout_remove_grid_gutter ~ '-0']) %} {% endif %}
Sure.. I might optimize this using some kind of translate_zf_bs_class_xx() functions, but yeah, still feels dirty.
- 🇩🇪Germany Anybody Porta Westfalica
Indeed both ways are not perfect, but changing the technical keys will mean many days of programming work. Proof me wrong, but these values are not only part of our layout paragraph settings but also of layouts used for other parts of the page potentially.
Finding these configs and updating them programmatically means hell through all these levels because Drupal provides no logics at all to do this with nice DX.
So in the end it's not perfect, but a lot safer to do leave the keys untouched.
But a good way might be to use hook_preprocess to map the values? Best of both worlds?
Proof me wrong, but these values are not only part of our layout paragraph settings but also of layouts used for other parts of the page potentially.
I don't understand what you mean, but... as I wrote above, I accept the fact that doing this in PHP is inefficient. So basically there is no choice in the current situation. So we have to do it in Twig.
- 🇩🇪Germany Anybody Porta Westfalica
PHP is not the point, these values are written into various parts of the Drupal config and there's no good way to find these places to update the old values. And keeping the old ones would lead to different implementations, which makes it even more complicated.
Why not use hook_preprocess? We could put that into the submodule and keep the templates clean!
Okay, I'll specify my statement: I accept that the backend part cannot be customized efficiently enough.
Why not use hook_preprocess? We could put that into the submodule and keep the templates clean!
Not sure if this helps. Various configuration fields that need to be translated and passed as custom variables to Twig. I'll think about it.
- 🇩🇪Germany Anybody Porta Westfalica
I think that might work well, as for foundation we could just keep things as-is and for bootstrap we would make these mappings:
'layout_align_cells_horizontal' => 'start', 'layout_remove_grid_gutter' => '', 'extra_classes' => '', ]; '#type' => 'select', '#title' => $this->t('Children vertical alignment'), '#options' => [ 'start' => $this->t('Top'), 'center' => $this->t('Middle'), 'end' => $this->t('Bottom'), 'stretch' => $this->t('Stretch'), ], '#default_value' => $configuration[$field_name], '#type' => 'select', '#title' => $this->t('Children horizontal alignment'), '#options' => [ 'start' => $this->t('Left'), 'center' => $this->t('Center'), 'end' => $this->t('Right'), 'between' => $this->t('Aligned to the edges (justify)'), 'around' => $this->t('Aligned to the space around (spaced)'), ], '#default_value' => $configuration[$field_name], '#empty_option' => $this->t('- None -'), '#type' => 'select', '#title' => $this->t('Remove grid spaces'), '#options' => [ 'lg' => $this->t('Large'), 'md' => $this->t('Medium'),
as switch statements in hook_preprocess in the boostrap drowl_layouts submodule for the config key.
But of course I could miss something. I think we could then leave the twig files untouched, but instead of changing the form, add the mapping, with the same result to twig? Ok, got it. But many Foundation/Bootstrap classnames are hardcoded in the Twig files, so we need to modify the templates, no matter how we solve the dynamic values.
- 🇩🇪Germany Anybody Porta Westfalica
Yes, I meant preprocess just for these mappings. Not for other classes.
- Issue was unassigned.
- Status changed to Closed: won't fix
about 1 year ago 8:41pm 7 March 2024 - 🇩🇪Germany Anybody Porta Westfalica