Skip to content

Conversation

@louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented Aug 1, 2023

Description

With git bisect, it seems the breaking change appeared from #37438.
The only change in here was that the parseSelector was applied in any case and not on some specific cases anymore. Reverting this change.

Motivation & Context

Break some (wrong?) use cases.

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Tried it with the minimal code:

<button data-bs-toggle="collapse" data-bs-target="#hello,#world">
  collapse
</button>

<div id="hello" class="collapse">HELLO</div>
<div id="world" class="collapse">WORLD</div>

Related issues

Closes #38974.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 1, 2023

Nice git bisecting!

Confirmed it fixes #38974. @GeoSot LGTY?

@XhmikosR
Copy link
Member

XhmikosR commented Aug 1, 2023

As for the tests, yeah, ideally we should have one, because apparently it's not covered by the existent tests :)

Copy link

@Parasuramragu Parasuramragu left a comment

Choose a reason for hiding this comment

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

its an clear review in this program
I had checked the program in this compailer in this program and i have check the program in VS Code

@XhmikosR XhmikosR requested a review from GeoSot August 22, 2023 08:27
Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

Nice analysis 💪 LGTM!

@julien-deramond julien-deramond merged commit 9900cf3 into main Sep 13, 2023
@julien-deramond julien-deramond deleted the main-lmp-fix-multi-ids branch September 13, 2023 07:30
@XhmikosR XhmikosR changed the title Collapse: Fix multiple ids calls Collapse: Fix multiple ids calls Sep 14, 2023
GeoSot added a commit that referenced this pull request Sep 17, 2023
@GeoSot GeoSot mentioned this pull request Sep 17, 2023
10 tasks
GeoSot added a commit that referenced this pull request Sep 18, 2023
romankupchak93 pushed a commit to romankupchak93/bootstrap that referenced this pull request Jan 5, 2024
GeoSot added a commit that referenced this pull request Feb 9, 2024
GeoSot added a commit that referenced this pull request Feb 13, 2024
XhmikosR added a commit that referenced this pull request Feb 18, 2024
* fix: regression of #38989

* Add unit test in selector-engine.spec.js

---------

Co-authored-by: Julien Déramond <juderamond@gmail.com>
Co-authored-by: XhmikosR <xhmikosr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

Collapse with multiple target stopped working in 5.3.0

5 participants