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

ngbDropdownToggle double triggers on child element #803

Closed
karptonite opened this issue Sep 27, 2016 · 3 comments
Closed

ngbDropdownToggle double triggers on child element #803

karptonite opened this issue Sep 27, 2016 · 3 comments

Comments

@karptonite
Copy link

karptonite commented Sep 27, 2016

Bug description:

When you have the ngbDropdownToggle on a button element, and a span inside that button element, clicking on the span (inside the button) does not toggle the dropdown (or, more specifically, I think it double-toggles it back to its original state).

This bug is new as of alpha 6--it may be related to the change from HostListener to (click) for handling the event. The bug was observed using chrome version 53.

Link to minimally-working plunker that reproduces the issue:

http://plnkr.co/edit/TyaAkbqHeLq7HidFzpNf?p=preview

Version of Angular, ng-bootstrap, and Bootstrap:

Angular: 2.0.0? whatever the sample plunker defaults to--it does not specify

ng-bootstrap: 1.0.0-alpha.6

Bootstrap: 4.0.0-alpha.4

@pkozlowski-opensource
Copy link
Member

@karptonite thnx for reporting, this is indeed a bug on our side (luckily with an easy fix!). What needs changing is this line to listen for child elements as well (this._toggleElement.contains).

A new test should be added, similar to the existing one:

it('should toggle dropdown on click', () => {
const html = `
<div ngbDropdown>
<button ngbDropdownToggle>Toggle dropdown</button>
</div>`;
const fixture = createTestComponent(html);
const compiled = fixture.nativeElement;
let dropdownEl = getDropdownEl(compiled);
let buttonEl = compiled.querySelector('button');
expect(dropdownEl).not.toHaveCssClass('open');
expect(buttonEl.getAttribute('aria-haspopup')).toBe('true');
expect(buttonEl.getAttribute('aria-expanded')).toBe('false');
buttonEl.click();
fixture.detectChanges();
expect(dropdownEl).toHaveCssClass('open');
expect(buttonEl.getAttribute('aria-expanded')).toBe('true');
buttonEl.click();
fixture.detectChanges();
expect(dropdownEl).not.toHaveCssClass('open');
expect(buttonEl.getAttribute('aria-expanded')).toBe('false');
});

PRs welcomed!

@JNKielmann
Copy link

Is anyone working on this? Otherwise I would try to make a PR.

@pkozlowski-opensource
Copy link
Member

@JNKielmann looks like we've got a PR for this one already #810

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

Successfully merging a pull request may close this issue.

3 participants