Skip to content
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

FormRow: allow attributes for <fieldset> wrapper #62

Open
Slamdunk opened this issue Apr 1, 2020 · 3 comments · May be fixed by #253
Open

FormRow: allow attributes for <fieldset> wrapper #62

Slamdunk opened this issue Apr 1, 2020 · 3 comments · May be fixed by #253

Comments

@Slamdunk
Copy link
Contributor

Slamdunk commented Apr 1, 2020

Q A
New Feature yes
RFC no
BC Break no

Hi, so far the <fieldset> wrapper for multi-checkbox, radio and so on can't have attributes since the rendered html is hard-coded:

if ($type === 'multi_checkbox'
|| $type === 'radio'
|| $element instanceof MonthSelect
|| $element instanceof Captcha
) {
$markup = sprintf(
'<fieldset><legend>%s</legend>%s</fieldset>',
$label,
$elementString
);

For the multi-checkbox, an hypothetical $multiCheckbox->setAttribute('fieldset_class' => 'foo'); could be retrieved and injected: we should think of a comprehensive solution for all the involved elements though.

@froschdesign
Copy link
Member

@Slamdunk
Thanks for this request!

For the multi-checkbox, an hypothetical $multiCheckbox->setAttribute('fieldset_class' => 'foo'); could be retrieved and injected: we should think of a comprehensive solution for all the involved elements though.

The question which arise here: is the FormRow helper the correct place to handle the fieldset?

The helper FormCollection already contains a "wrapper" functionality. Maybe we can adopt this functionality to all helper with multiple elements and also for the FormRow helper itself. This would allow to create custom wrappers for the different element types and to create a markup with the FormRow helper which is used by CSS frameworks like Bootstrap, Tailwind and Co.

Please attach further ideas and thoughts!

@Slamdunk
Copy link
Contributor Author

Slamdunk commented Apr 1, 2020

The helper FormCollection already contains a "wrapper" functionality. Maybe we can adopt this functionality to all helper with multiple elements and also for the FormRow helper itself.

That's indeed a good suggestion 👍

[...] to create a markup with the FormRow helper which is used by CSS frameworks like Bootstrap, Tailwind and Co

In fact that's what I was looking for: avoid custom styling and rely solely to external CSS tools. This should be the path for any view helper these days: classes allowed everywhere 🚢

@froschdesign
Copy link
Member

I think the goal for standard forms should be:

<?= $this->form($form) ?>

which means we need support for configurations like this:

return [
    'view_helper_config' => [
        'form_row' => [
            'input_error_class' => 'is-invalid',
            'wrapper'           => '…',
            'label_wrapper'     => '…',
        ],
    ],
];

or (pseudo code)

return [
    'view_helper_config' => Laminas\Form\View\Helper\HelperConfig::BOOTSTRAP,
];
return [
    'view_helper_config' => Laminas\Form\View\Helper\HelperConfig::FOUNDATION,
];

Compare also with Laminas\View\Helper\Service\FlashMessengerFactory:

https://github.com/laminas/laminas-view/blob/b74443fd42490caf2b92959f8867fb232f8ac52f/src/Helper/Service/FlashMessengerFactory.php#L38-L50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants