Skip to content

[Toolkit] WIP: [Bootstrap] Add Bootstrap 5#3463

Open
BafS wants to merge 5 commits intosymfony:3.xfrom
BafS:kit-bootstrap-1
Open

[Toolkit] WIP: [Bootstrap] Add Bootstrap 5#3463
BafS wants to merge 5 commits intosymfony:3.xfrom
BafS:kit-bootstrap-1

Conversation

@BafS
Copy link
Copy Markdown
Contributor

@BafS BafS commented Apr 13, 2026

Q A
Bug fix? no
New feature? yes
Deprecations? no
Documentation? yes
Issues no
License MIT

Add bootstrap 5.3 kit with multiple components. There is still more components to do but I think it's worth having feedback before I continue. For disclosure, I used Claude Opus 4.6 to help me.

@BafS BafS requested a review from Kocal as a code owner April 13, 2026 22:21
@carsonbot carsonbot added Documentation Improvements or additions to documentation Feature New Feature Toolkit Status: Needs Review Needs to be reviewed Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Apr 13, 2026
@carsonbot carsonbot changed the title WIP: [Toolkit][Bootstrap] Add Bootstrap 5 [Toolkit] WIP: [Bootstrap] Add Bootstrap 5 Apr 13, 2026
@DcgRG
Copy link
Copy Markdown
Contributor

DcgRG commented Apr 14, 2026

Hello, that's great, a new kit!

To start with a quick overview, here are two small points:

  • For conditionally applying your classes, I recommend using html_cva, which will make the templates more readable.
  • For interactive components, I think it might be beneficial to keep the trigger pattern from other kits so you have full control over the elements that trigger interactions.

Bonus and purely personal opinion: for interactive elements like collapse, rather than using data attributes, I think it might be more efficient to use a stimulus controller that then uses a Bootstrap object (e.g., new bootstrap.Collapse(collapseEl)).

@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Apr 14, 2026
@BafS
Copy link
Copy Markdown
Contributor Author

BafS commented Apr 14, 2026

Thanks for the feedback @DcgRG, I tried to address those


connect() {
this._collapse = new Collapse(this.contentTarget, {
toggle: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can add "parent" option here

@DcgRG
Copy link
Copy Markdown
Contributor

DcgRG commented Apr 15, 2026

Ultimately, is an Accordion component simply a group of Collapse elements?
You can use your generic collapse element to create your Accordion component; this could save you from creating an additional controller for Accordion.

Nope ?

@BafS
Copy link
Copy Markdown
Contributor Author

BafS commented Apr 15, 2026

Indeed, the accordion is a map of collapse. I removed the accordion controller. It's just a little bit confusing to have things like data-action="click->collapse#toggle" in the accordion I feel, but it's probably a detail.

@BafS BafS force-pushed the kit-bootstrap-1 branch from dd9d18f to ee03afe Compare April 15, 2026 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Improvements or additions to documentation Feature New Feature Status: Needs Review Needs to be reviewed Toolkit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants