Fix dropdown escape propagation

[alpadev]

Closes #33465

[GeoSot]

LGTM

[rohit2sharma95]

bootstrap/js/src/dropdown.js

Lines 489 to 492 in 5cc53a0

	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
}

[alpadev]

bootstrap/js/src/dropdown.js

Lines 489 to 492 in 5cc53a0

	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.

bootstrap/js/src/dropdown.js

Lines 458 to 464 in 5cc53a0

	if (/input|textarea/i.test(event.target.tagName) ?
	event.key === SPACE_KEY || (event.key !== ESCAPE_KEY &&
	((event.key !== ARROW_DOWN_KEY && event.key !== ARROW_UP_KEY) ||
	event.target.closest(SELECTOR_MENU))) :
	!REGEXP_KEYDOWN.test(event.key)) {
	return
	}

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

bootstrap/js/src/dropdown.js

Line 44 in 5cc53a0

const REGEXP_KEYDOWN = new RegExp(`${ARROW_UP_KEY}|${ARROW_DOWN_KEY}|${ESCAPE_KEY}`)

[rohit2sharma95]

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

/CC @MartijnCuppens

[rohit2sharma95]

And can not the selectMenuItem 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 the dataApiKeydownHandler method is bound with the dropdown elements only (menu and the toggle button).

[alpadev]

And can not the selectMenuItem 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 the dataApiKeydownHandler 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 way dataApiKeydownHandler was implemented.

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

[GeoSot]

Till now is just a refactoring that I can follow, a minor fix and a valid test that succeeds . So till here I find it OK.

I agree to refactor selectMenuItem to private method, and would also love to see dataApiKeydownHandler 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