Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Preventing event from a button in an accordion header refreshes the page #3299

Closed
bgever opened this issue Feb 16, 2015 · 7 comments
Closed

Comments

@bgever
Copy link
Contributor

bgever commented Feb 16, 2015

Version 0.12 introduced a problem with my use case. I have an accordion, and in its header I have several buttons. When clicking these buttons, the default behavior is to execute the action of that button and toggling the body content.

In order to prevent this, I capture the $event in the binding, and call its stopPropagation() in the button handler.

As of version 0.12 this causes a refresh of the page.
See the bug reproduced in Plunker.

I've found the problem to be originating from commit 992b232, where a href attribute was added to the header, in order for the keyboar navigation to focus on the link element. However, in turn, the browser will hook up a default handler to navigate to the empty URL. This navigation happens when the $event is stopped propagating, like in my example.

I've found the solution to be to attach a noop to the href attribute; href="javascript:void(0)". This keeps the keyboard navigation working, and when stopping the event, still has the expected behavior of just cancelling the accordion toggle.

bgever added a commit to bgever/bootstrap that referenced this issue Feb 16, 2015
…nested buttons canceling events

When nesting buttons in the header, and calling `$event.stopPropagation()` in their click handlers would cause a page refresh, because the default empty URL would be executed. By changing this into a noop, the page refresh is prevented. angular-ui#3299
wesleycho pushed a commit that referenced this issue Mar 15, 2015
…nested buttons canceling events

When nesting buttons in the header, and calling `$event.stopPropagation()` in their click handlers would cause a page refresh, because the default empty URL would be executed. By changing this into a noop, the page refresh is prevented. #3299
@chrisirhc
Copy link
Contributor

@bgever You need to add a $event.preventDefault() call if you want to stopPropagation. That prevents the default action of the click (to navigate, in the case of an anchor element with href). By default, Angular does this for you but you've done stopPropagation so it can't do it.

See:
http://plnkr.co/edit/CJ30XW83dqC7U7u7twUM?p=preview

@chrisirhc
Copy link
Contributor

Closing as it's resolved via #3300 which has been merged.

@karianna karianna added this to the 0.13.0 milestone Mar 22, 2015
@bgever
Copy link
Contributor Author

bgever commented Mar 23, 2015

@chrisirhc I've avoided the $event.preventDefault() call, because this might imply the wrong behavior. The nested button click itself should not be cancelled (think of multiple event handlers).

Thanks for pointing this solution, it looks to work great with versions before 0.13.

GabrielDelepine pushed a commit to GabrielDelepine/bootstrap that referenced this issue May 7, 2015
…nested buttons canceling events

When nesting buttons in the header, and calling `$event.stopPropagation()` in their click handlers would cause a page refresh, because the default empty URL would be executed. By changing this into a noop, the page refresh is prevented. angular-ui#3299

Conflicts:
	template/accordion/accordion-group.html
@noullet
Copy link

noullet commented Oct 20, 2015

Hi,

The issue is back in 0.13.3 and above.

It seems that it has been brought back by fixing #4104, but this issue seems to be back too.

Here are the examples updated to 0.14.2

For the record, I want to do exactly the same thing as in #4104: when the user clicks on the accordion header button, the accordion is not opened, the page is not refreshed to the current location and the user is redirected to a new location.

It should also be noted that the first fix for the current issue caused #3904 and the second fix caused #4029 so a new fix should ensure that these related issues stay fixed.

@wesleycho
Copy link
Contributor

Problem is, they are sort of diametrically opposed here - one fix will cause another. On the user's side, one way to fix this is to add a $event.stopPropagation call on the header element, if I recall correctly.

There some very funky behavior with how the browser responds in this exact situation.

@noullet
Copy link

noullet commented Oct 20, 2015

Actually the workaround suggested by @chrisirhc appears to work for me:

<uib-accordion-heading>
    ...
    <button ng-click="$event.stopPropagation();$event.preventDefault();openNewLocation()">
        ...
    </button>
</uib-accordion-heading>

I lost some time trying to figure out this funky mess and that I wanted to document my findings, including the fact that two issues marked as closed are still there, hence my comment.

@AnthonyNahas
Copy link

I went through the same problem and really the workaround above worked for me !!!

with refresh page

<button fxHide.gt-sm="true"
                            color="primary"
                            mat-raised-button
                            (click)="onTemplateSelected.next(template)">
                        save </button>

without refresh page

<button fxHide.gt-sm="true"
                            color="primary"
                            mat-raised-button
                            (click)="$event.stopPropagation();$event.preventDefault();onTemplateSelected.next(template)">
                        save </button>

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

No branches or pull requests

6 participants