Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropdown: jQuery throws with empty ID selector #21328

Closed
gpakosz opened this issue Dec 12, 2016 · 14 comments
Closed

Dropdown: jQuery throws with empty ID selector #21328

gpakosz opened this issue Dec 12, 2016 · 14 comments

Comments

@gpakosz
Copy link
Contributor

gpakosz commented Dec 12, 2016

Hello,

While following the documentation, I've stumbled upon Dropdown examples: http://getbootstrap.com/javascript/#via-data-attributes-1

<div class="dropdown">
  <a id="dLabel" data-target="#" href="http://example.com" data-toggle="dropdown" role="button" aria-haspopup="true" aria-expanded="false">
    Dropdown trigger
    <span class="caret"></span>
  </a>

  <ul class="dropdown-menu" aria-labelledby="dLabel">
    ...
  </ul>
</div>

When trying it with jQuery v2.1.3, everything works fine and clicking the anchor displays the dropdown.

However, when using jQuery v3.1.0 clicking the anchor follows the link.

@bardiharborow
Copy link
Member

Support for v3 has ceased. Please let us know if your issue affects v4.

@gpakosz
Copy link
Contributor Author

gpakosz commented Dec 13, 2016

Hmm with v4 it seems markup is totally different anyways

@mdo
Copy link
Member

mdo commented Dec 13, 2016

If this isn't affecting v4, we'll close it out as a won't fix.

@gpakosz
Copy link
Contributor Author

gpakosz commented Dec 13, 2016

This is what I'm experiencing with v4:

I appreciate that v4 dropdowns documentation doesn't mention data-target="#" while v3 documentation used to. However, I would say people migrating from v3 to v4 will face that situation as the migration guide doesn't instruct to remove data-target="#" in that case.

In that respect, can you please enlighten me about:

  • what's the current or past role of data-target="#" exactly?
  • what are the tradeoffs between using <button> vs <a> for dropdown togglers?

@Johann-S
Copy link
Member

Johann-S commented Dec 13, 2016

As it said in V3 documentation :

To keep URLs intact with link buttons, use the data-target attribute instead of href="#".

And the difference between <button> and <a> depends on your needs, if you want to toggle the dropdown from a classic link or from a button

@gpakosz
Copy link
Contributor Author

gpakosz commented Dec 13, 2016

Should one understand that "keeping the URLs intact" mean clicking the link toggler won't toggle the dropdown?

About the difference between <button> and <a> as far as I can tell, using either produces the same rendering. That's why I was asking whether there's subtle difference I overlooked.

@Johann-S
Copy link
Member

IMO "keeping the URLs intact" means to not add # or any anchors to the end of the URL but the dropdown have to be toggled.

Hmm yeah my bad on the difference between <button> and <a> maybe they are a compatibility reason I'm not sure.

@hnrch02
Copy link
Collaborator

hnrch02 commented Jan 1, 2017

what's the current or past role of data-target="#" exactly?

Its role is to select the dropdown menu that the button or link should toggle by ID. If you don't use data-target it will use the value of the href attribute. This makes it possible to place the toggler in a different element than the dropdown itself.

jQuery produces an error in v3 because # is no longer a valid selector and that's why the behavior described here occurs. We need to fix this.

what are the tradeoffs between using vs for dropdown togglers?

A button is semantically more correct, the link is most often found in navbars. If you don't need to have an anchor, I would always use a button. Maybe someone else can weigh in on this though since I'm not the expert on these kinds of questions.

@hnrch02 hnrch02 added confirmed v4 and removed v3 labels Jan 1, 2017
@hnrch02 hnrch02 added this to the v4.0.0-alpha.6 milestone Jan 1, 2017
@hnrch02 hnrch02 changed the title Dropdowns, links & jQuery v3.1.0 Dropdown: jQuery throws with empty ID selector Jan 1, 2017
@hnrch02
Copy link
Collaborator

hnrch02 commented Jan 1, 2017

X-Ref: #21341, #20019

@mdo mdo modified the milestones: v4.0.0-alpha.6, v4.0.0-beta Jan 4, 2017
@Johann-S
Copy link
Member

Johann-S commented Jan 6, 2017

Sorry @hnrch02 but it seems it doesn't appear in V4, it's the case in V3

But as @gpakosz said :

However, I would say people migrating from v3 to v4 will face that situation as the migration guide doesn't instruct to remove data-target="#" in that case.

@hnrch02
Copy link
Collaborator

hnrch02 commented Jan 8, 2017

@Johann-S How does it not appear in v4? The error occurs in this Bin.

Even though it appears that data-target is no longer documented, the functionality still exists in the v4 codebase and works just fine.

@Johann-S
Copy link
Member

Johann-S commented Jan 8, 2017

Oh ok you're right sorry @hnrch02

@mdo mdo added the has-pr label Jan 9, 2017
pvdlg added a commit to pvdlg/bootstrap that referenced this issue Jan 11, 2017
pvdlg added a commit to pvdlg/bootstrap that referenced this issue Jan 27, 2017
* collapse-multiple-target:
  Use $(document).find(selector) to avoid case in twbs#20184
  Muti-target support for collapse plugin
  make getTargets to always return a JQuery to avoid calling JQuery on the same element further down
  Add a dropdown test case for twbs#21328
  Simplify targets.length test
  Simplify null check when possible
  Rework getSelectorFromElement to not rely on regex

# Conflicts:
#	js/src/alert.js
#	js/src/dropdown.js
#	js/tests/unit/collapse.js
derwok added a commit to 4minitz/4minitz that referenced this issue Mar 5, 2017
After migration to jQuery 3.x we got lots of client side exceptions caused by not rendering navbar dropdown. This is a known twitter bootstrap 3 problem:
twbs/bootstrap#21328


The solution seems to be simply to remove the data-target="#" from the dropdown trigger <a>
@Alecto
Copy link

Alecto commented Oct 17, 2017

i have the same error on drupal 8.4 and jquery 3.2.1 build-in by default, where i use bootsrap 3.3.7.
is there any way to fix this error on bootsrap 3.3.7? i cannot use v4

@Lendude
Copy link

Lendude commented Nov 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants