yiisoft / yii2-bootstrap4

Yii 2 Bootstrap 4 Extension
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
216 stars 106 forks source link

Avoiding collisions with automatically generated IDs in ToggleButtonGroup #181

Closed marcovtwout closed 1 year ago

marcovtwout commented 4 years ago

What steps will reproduce the problem?

BaseHtml automatically generates ID's for boolean() inputs (like radio and checkbox) with a label: https://github.com/yiisoft/yii2-bootstrap4/blob/master/src/BaseHtml.php#L146 This is similar to the mechanism Yii core uses when automatically generating ID's for widgets.

In Yii core, it's a best practice to set explicit IDs for widgets when dealing with ajax requests to avoid collisions. But ToggleButtonGroup does not allow or apply this principle. https://github.com/yiisoft/yii2-bootstrap4/blob/master/src/ToggleButtonGroup.php#L128

When rendering these inputs with ajax requests and combining those into a single HTML page, duplicate IDs will occur (multiple inputs with id="i0").

What's expected?

ToggleButtonGroup could pass smarter ID's similar to what Yii 1's radiobuttonlists did, to avoid those collisions (https://github.com/yiisoft/yii/blob/master/framework/web/helpers/CHtml.php#L1271).

public function renderItem($index, $label, $name, $checked, $value)
{
    (...)
    return Html::$type($name, $checked, [
        (...)
        'id' => Html::getInputIdByName($name) . '_' $index // getInputByName() does not exist yet, but see getInputId() for reference
    ]);

Which generated inputs with IDs like: id="myRadio_0", id="myRadio_1")

samdark commented 4 years ago

That sounds like a good thing to fix but won't it change IDs generated slightly?

marcovtwout commented 4 years ago

Are you worried about backwards compatibility? The proposed solution will change ID's for ToggleButtonGroup. But they will still be (more) unique.

samdark commented 4 years ago

Are you worried about backwards compatibility?

Yes. We can release it as 2.1.0 though.

samdark commented 4 years ago

@marcovtwout want to do a pull request?

marcovtwout commented 4 years ago

@samdark Sure. Would you put getInputIdByName() in yii2-bootstrap4's BaseHtml?

samdark commented 4 years ago

Yes, why not.

marcovtwout commented 4 years ago

@samdark Because getInputIdByName() partly duplicates getInputId() available in yii core. I suppose getInputIdByName() could be moved up and duplicate code extracted, but modifying yii core code is beyond the scope of this PR.

marcovtwout commented 3 years ago

getInputIdByName() is now available in next yii core release, PR https://github.com/yiisoft/yii2-bootstrap4/pull/182 updated