search

yiisoft / yii-bootstrap5

Yii Framework Bootstrap 5 support
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
63 stars 19 forks source link

fix to render Stringable content in Accordion item #85

Closed albertborsos closed 2 years ago

albertborsos commented 2 years ago
Q A
Is bugfix? ✔️
New feature? ❌
Breaks BC? ❌
Fixed issues https://github.com/yiisoft/yii-bootstrap5/commit/e656fe39620102ad90ab33194dd29136e912808f#r70189673
vjik commented 2 years ago

@albertborsos thank you for PR!

Can you add test for this change, please?

albertborsos commented 2 years ago

@vjik I made it, but there is something to discuss.

In my test cases an .accordion-item has only one .accordion-body. But in the previous tests there is multiple .accordion-body in an .accordion-item.

I think the previous tests are uses a wrong accordion syntax.

albertborsos commented 2 years ago

FYI: on php7.4 a \Yiisoft\Html\Tag\Base\Tag instance is not a Stringable object, even if symfony/polyfill-php80 is installed.

vjik commented 2 years ago

@albertborsos We can check exists of method __toString() instead of is_a(). This is resolve problem with stringable objects.

In my test cases an .accordion-item has only one .accordion-body. But in the previous tests there is multiple .accordion-body in an .accordion-item. I think the previous tests are uses a wrong accordion syntax.

@terabytesoftw What you think about this? Item can have several body?

terabytesoftw commented 2 years ago

@albertborsos We can check exists of method __toString() instead of is_a(). This is resolve problem with stringable objects.

In my test cases an .accordion-item has only one .accordion-body. But in the previous tests there is multiple .accordion-body in an .accordion-item. I think the previous tests are uses a wrong accordion syntax.

@terabytesoftw What you think about this? Item can have several body?

https://getbootstrap.com/docs/5.1/components/accordion/ if we go strictly to the documentation, it only has a body, but use the array as if it were a string varadic which is more convenient than concatenating everything, https://github.com/yiisoft/yii-bootstrap5/blob/e656fe39620102ad90ab33194dd29136e912808f/tests/AccordionTest.php#L56