yii2-bootstrap4 icon indicating copy to clipboard operation
yii2-bootstrap4 copied to clipboard

Avoiding collisions with automatically generated IDs in ToggleButtonGroup

Open marcovtwout opened this issue 5 years ago • 8 comments

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")

marcovtwout avatar Jan 09 '20 14:01 marcovtwout

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

samdark avatar Jan 13 '20 09:01 samdark

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

marcovtwout avatar Jan 14 '20 08:01 marcovtwout

Are you worried about backwards compatibility?

Yes. We can release it as 2.1.0 though.

samdark avatar Jan 14 '20 10:01 samdark

@marcovtwout want to do a pull request?

samdark avatar Jan 14 '20 10:01 samdark

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

marcovtwout avatar Jan 14 '20 11:01 marcovtwout

Yes, why not.

samdark avatar Jan 14 '20 11:01 samdark

@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 avatar Jan 24 '20 16:01 marcovtwout

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

marcovtwout avatar May 14 '21 08:05 marcovtwout