LGTM

bootstrap/js/src/dropdown.js

Lines 489 to 492 in5cc53a0

	if(!isActive||event.key===SPACE_KEY){
	Dropdown.clearMenus()
	return
	}

This should not be changed as below? 🤔

if(isActive&&event.key===SPACE_KEY){Dropdown.clearMenus()return}

@rohit2sharma95 Not sure if this is even needed.

This checks for keys that are dropdown commands.

But the regex is missing the space key so I'm not sure if we even reach this block.

This change was made in#29885 (and then in#30597, that's unrelated though). Anything related to input groups?

/CC@MartijnCuppens

And can not theselectMenuItem method be a private method instead of a static one (as it is not being used directly 🤔)? It can be used through the context of the dropdown then IMO. Because thedataApiKeydownHandler method is bound with the dropdown elements only (menu and the toggle button).

And can not theselectMenuItem method be a private method instead of a static one (as it is not being used directly 🤔)? It can be used through the context of the dropdown then IMO. Because thedataApiKeydownHandler method is bound with the dropdown elements only (menu and the toggle button).

I didn't mean to refactor too much in the scope of this issue but rather to just get rid of that Eslint warning. I agree that using a static method isn't ideal but the item selection functionality wasn't bound to the instance before and I made it the waydataApiKeydownHandler was implemented.

If you want me to change it I should be able to do this tho.

Till now is just a refactoring that I can follow, a minor fix and a valid test that succeeds . Sotill here I find itOK.

I agree to refactorselectMenuItem to private method, and would also love to seedataApiKeydownHandler gordian knot untied, but on another PR, to avoid mistakes

So if we can all agree on these, I would prefer to keep it as it is and if@alpadev has the courage to keep up with a following PR