yiisoft / yii-bootstrap5

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

Collapse improvements #127

Closed Gerych1984 closed 1 year ago

Gerych1984 commented 1 year ago
Q A
Is bugfix?
New feature? ✔️
Breaks BC? ✔️
  1. New Collapse widget
  2. New AbstractToggleWidget for widgets with toggler
  3. New AbstractCloseButtonWidget for widgets with optional close button
  4. Accordion and NavBar now using Collapse instead of generate html tags
  5. Replace NavBar::offcanvas(?Offcanvas $offcanvas) to NavBar::withWidget(Offcanvas|Collapse|null $widget)
  6. Collapse and NavBar now extended from AbstractToggleWidget
  7. Modal and Offcanvas now extended from AbstractCloseButtonWidget
what-the-diff[bot] commented 1 year ago

PR Summary

This Pull Request provides significant updates making the code more efficient and organized, simplifying several classes' functioning and integrating improved test scenarios for better development and potential troubleshooting.

codecov[bot] commented 1 year ago

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (e74b93e) 97.61% compared to head (5ebae88) 98.46%.

:exclamation: Current head 5ebae88 differs from pull request most recent head 9ffc0a2. Consider uploading reports for the commit 9ffc0a2 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #127 +/- ## ============================================ + Coverage 97.61% 98.46% +0.84% - Complexity 472 497 +25 ============================================ Files 18 21 +3 Lines 1637 1692 +55 ============================================ + Hits 1598 1666 +68 + Misses 39 26 -13 ``` | [Files](https://app.codecov.io/gh/yiisoft/yii-bootstrap5/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft) | Coverage Δ | | |---|---|---| | [src/AbstractToggleWidget.php](https://app.codecov.io/gh/yiisoft/yii-bootstrap5/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft#diff-c3JjL0Fic3RyYWN0VG9nZ2xlV2lkZ2V0LnBocA==) | `100.00% <100.00%> (ø)` | | | [src/Modal.php](https://app.codecov.io/gh/yiisoft/yii-bootstrap5/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft#diff-c3JjL01vZGFsLnBocA==) | `98.15% <100.00%> (+1.44%)` | :arrow_up: | | [src/NavBar.php](https://app.codecov.io/gh/yiisoft/yii-bootstrap5/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft#diff-c3JjL05hdkJhci5waHA=) | `100.00% <100.00%> (ø)` | | | [src/Offcanvas.php](https://app.codecov.io/gh/yiisoft/yii-bootstrap5/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft#diff-c3JjL09mZmNhbnZhcy5waHA=) | `98.85% <100.00%> (+3.95%)` | :arrow_up: | | [src/AbstractCloseButtonWidget.php](https://app.codecov.io/gh/yiisoft/yii-bootstrap5/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft#diff-c3JjL0Fic3RyYWN0Q2xvc2VCdXR0b25XaWRnZXQucGhw) | `96.66% <96.66%> (ø)` | | | [src/Accordion.php](https://app.codecov.io/gh/yiisoft/yii-bootstrap5/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft#diff-c3JjL0FjY29yZGlvbi5waHA=) | `98.38% <97.67%> (+4.63%)` | :arrow_up: | | [src/Collapse.php](https://app.codecov.io/gh/yiisoft/yii-bootstrap5/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft#diff-c3JjL0NvbGxhcHNlLnBocA==) | `98.75% <98.75%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Gerych1984 commented 1 year ago

New abstract classes more like traits. Suggest refactor to traits. @Gerych1984 what do you think?

I've been thinking about it. There are a few nuances

  1. Need an abstract method with the name of the component (bsTarget - traits can set such)
  2. Need to be sure that the trait is used in a class that implements the id() method to set the data-bs-target attribute for the switch (Add another abstract method to it?).
  3. Right now I can't check if it's possible to create a close button outside the widget. But if it is (and that's why I made its button renderer public), then this trait will need id() method too
  4. For bc i used protected properties and replace it in some widgets (Like $toggleLabel = 'Show' by default in Modal). With traits that would be problematic.
samdark commented 1 year ago

👍