Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Add a missing way to set the HTML attributes of the fieldset tag #5952

Closed
wants to merge 1 commit into from
Closed

Conversation

ZeinEddin
Copy link
Contributor

Add a missing way to set the HTML attributes of the fieldset tag as reported in #5553

@@ -171,7 +172,8 @@ public function render(ElementInterface $element)
$type = $element->getAttribute('type');
if ($type === 'multi_checkbox' || $type === 'radio') {
$markup = sprintf(
'<fieldset><legend>%s</legend>%s</fieldset>',
'<fieldset %s><legend>%s</legend>%s</fieldset>',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace "<fieldset %s" with "<fieldset%s" and add the space if $attributes isn't empty.

See: fd62467#diff-59cad3b357bb99a8cf119367da595551R128

@grizzm0
Copy link
Contributor

grizzm0 commented Mar 12, 2014

The attributes are also passed to multi_checkbox and used for the child elements. Any attributes used here will be used twice. Even ID will be rendered twice which will result in invalid HTML.

While #5623 makes sense. This does not. FormRow adds the extra fieldset/legend to be able to render the group of elements correctly. It's not an ACTUAL fieldset you're rendering as in the case of FormCollection.

@Ocramius
Copy link
Member

Ocramius commented Apr 3, 2014

@ZeinEddin we need a test for this.

@weierophinney
Copy link
Member

Closing due to lack of feedback from original author; feel free to re-open if/when you can add unit tests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants