Skip to content

Update bundle.js in tests/integration#32233

Merged
XhmikosR merged 3 commits into
twbs:mainfrom
rohit2sharma95:XhmikosR-patch-7
Nov 23, 2020
Merged

Update bundle.js in tests/integration#32233
XhmikosR merged 3 commits into
twbs:mainfrom
rohit2sharma95:XhmikosR-patch-7

Conversation

@rohit2sharma95
Copy link
Copy Markdown
Contributor

Use Array.prototype.concat() instead of direct spreading nodelist.

I am not sure if it should be done in this file also @XhmikosR

Ref: #32227 (comment)

@XhmikosR
Copy link
Copy Markdown
Member

XhmikosR commented Nov 23, 2020

@rohit2sharma95 good catch! Yes, please change the other instance too.

It seems I caused this regression here and never noticed it until a couple of days ago :/

What I don't get is

  1. why this worked when I changed the script to dist/js/bootstrap.bundle.js.
  2. the carousel doesn't seem to work even with this change but I guess that's normal since we only reference tooltip. Makes me wonder if we should also fix the carousel or add a note

PS. Do you have an account on our Slack? If not, feel free to join and DM me :)

@rohit2sharma95
Copy link
Copy Markdown
Contributor Author

Tooltip is working after you changed the path of the script because it is being initialized here:

var tooltipTriggerList = [].slice.call(document.querySelectorAll('[data-bs-toggle="tooltip"]'))
var tooltipList = tooltipTriggerList.map(function (tooltipTriggerEl) {
return new bootstrap.Tooltip(tooltipTriggerEl)

And carousel seems working on my local machine 🤔

@rohit2sharma95
Copy link
Copy Markdown
Contributor Author

It is being done in #31178 though
Ref: #31178 (comment)

@XhmikosR
Copy link
Copy Markdown
Member

Yeah, but it shouldn't be sneaked it like that :)

I'd rather merge it separately. I'll have a look tomorrow.

@XhmikosR XhmikosR changed the base branch from XhmikosR-patch-7 to main November 23, 2020 19:35
@XhmikosR XhmikosR merged commit 7c7f08b into twbs:main Nov 23, 2020
@rohit2sharma95 rohit2sharma95 deleted the XhmikosR-patch-7 branch November 24, 2020 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants