bootstrap icon indicating copy to clipboard operation
bootstrap copied to clipboard

v5.2.1 dropdown TypeError

Open XhmikosR opened this issue 3 years ago • 3 comments

Prerequisites

Describe the issue

<div class="dropdown">
  <ul class="dropdown-menu">
    <li class="d-block d-lg-none">
      <h6 class="dropdown-header text-wrap">
        WHATEVER
      </h6>
    </li>
    <li>
      <hr class="dropdown-divider d-block d-md-none">
    </li>
    <li>
      <button class="dropdown-item" type="button">Logout</button>
    </li>
  </ul> <!-- .dropdown-menu -->
</div> <!-- .dropdown -->
Uncaught TypeError: this._menu is undefined
    _isShown bootstrap.bundle.js:4040
    toggle bootstrap.bundle.js:3910
    <anonymous> bootstrap.bundle.js:4241
    handler bootstrap.bundle.js:397
    invokeTask zone.js:406
    runTask zone.js:178
    invokeTask zone.js:487
    invokeTask zone.js:1661
    globalCallback zone.js:1704
    globalZoneAwareCaptureCallback zone.js:1729

which points to:

_isShown() {
  return this._menu.classList.contains(CLASS_NAME_SHOW$6);
}

Same markup works with 5.2.0.

Reduced test cases

See above

What operating system(s) are you seeing the problem on?

Windows, Android

What browser(s) are you seeing the problem on?

Chrome, Firefox

What version of Bootstrap are you using?

v5.2.1


Seems like https://github.com/twbs/bootstrap/commit/337068f8b1044004f4b9abfffbb433694ae87993 is to blame

XhmikosR avatar Sep 09 '22 08:09 XhmikosR

You are right on the https://github.com/twbs/bootstrap/commit/337068f8b1044004f4b9abfffbb433694ae87993, however according to docs the dropdowns are initialized on the .dropdown-toggle element (probably it would be better to initialized on parent, but this is another discussion)

As the .dropdown-toggle doesn't exist on the given markup, it is normal, to be initialized in a wrong way. So do you suggest keeping it as it is and support only the correct markup, or at least for 5v keep support for this too?

GeoSot avatar Sep 13 '22 22:09 GeoSot

Ah, I remember now. So, the main issue is that I didn't want the dropdown toggle icon so I just went with a plain old button and data-bs-toggle="dropdown". Now, I'm pretty sure I can't be the only one doing this even though it's not 100% correct.

So,

  1. this was a breaking change, even though the usage isn't 100% right
  2. we could maybe have a dropdown-toggle without the icon
  3. or we could change the code to look for data-bs-toggle="dropdown" instead of the dropdown-toggle class

I feel like the last solution would be the simplest, but it's been a while I looked at the code.

XhmikosR avatar Sep 15 '22 05:09 XhmikosR

Also, do note that we don't explicitly mention the dropdown-toggle class, but we do mention the data-bs-toggle="dropdown" attribute https://getbootstrap.com/docs/5.2/components/dropdowns/#usage

XhmikosR avatar Sep 15 '22 05:09 XhmikosR

@XhmikosR @GeoSot I do like the idea of it trigger only off of the data attribute instead of the .dropdown-toggle class. I'm good with any fix here, but that's a good hygiene thing for us to double check for v6. I personally think all JS behavior should be controlled by data attribute, and styling limited to classes. Let's discuss more in Slack and find a path forward :).

mdo avatar Sep 22 '22 05:09 mdo