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

Is there a way to stop propagation of events? #16105

Closed
chanan opened this issue Apr 30, 2018 · 17 comments
Closed

Is there a way to stop propagation of events? #16105

chanan opened this issue Apr 30, 2018 · 17 comments
Labels
area-blazor Includes: Blazor, Razor Components

Comments

@chanan
Copy link

chanan commented Apr 30, 2018

Is there a way in onclick (I guess using UIMouseEventArgs?) to stop the click from propagating?

@SteveSandersonMS
Copy link
Member

Not presently. We will add something along these lines if it becomes a common request though.

The main difficulty is that if we wanted to run the .NET code in a worker thread, then the bubbling and propagation decisions would all have to happen asynchronously.

@chanan
Copy link
Author

chanan commented May 1, 2018

Without it, having an onclick on an A tag doesn't seem to work

@SteveSandersonMS
Copy link
Member

@chanan Do you have repro steps? I just checked that the following both work (as in, they invoke the method):

<a onclick="@MyMethod">First tag</a>
<a href="http://example.com" onclick="@MyMethod">Second tag</a>

@chanan
Copy link
Author

chanan commented May 1, 2018

This doesn't work for me. It changes the location to "/":

@page "/test"

<a href="#" class="btn btn-secondary dropdown-toggle" role="button" aria-haspopup="true" aria-expanded="@IsOpen" onclick="@onclick">
    Dropdown
</a>

@functions {
    bool IsOpen { get; set; }
    void onclick()
    {
        IsOpen = !IsOpen;
        StateHasChanged();
        //Don't change location here only togle IsOpen
    }
}

@THplusplus
Copy link

Not being able to stop event propagation is pretty bad, especially inside a framework that encourages you to use nested components. There is simply no way to prevent unwanted events from bubbling up to parent components.

In the following example I have 3 buttons. My goal is to handle a click on any of these buttons directly in their onclick handlers, without the event bubbling up to the topmost <div> tag.

  1. Button 1 (JS) works as expected. It alerts "clicked!" followed by "event propagation stopped", and ends there
  2. Button 2 (C#) does not work at all. It alerts "event propagation stopped" first, while the button's onclick handler is never called
  3. Button 3 (C#) is half and half. First it alerts "BOOM" through the topmost <div>'s onclick handler which I wanted to prevent, then afterwards the buttons onclick handler is called, alerting "clicked!".

Currently the only idea I have to prevent this, is handling the event in JS only, and calling into C# from there. I don't even know yet how I am supposed to pass my component instance into that handler.

Definately a +1 from me for a Blazor way to stop event propagation.

<div onclick="alert('BOOM');">
    <div onclick="event.stopPropagation(); alert('event propagation stopped');">
        <button onclick="alert('clicked!');">Button 1 (JS)</button>
    </div>

    <div onclick="event.stopPropagation(); alert('event propagation stopped');">
        <button onclick="@((e) => Alert("clicked!"))">Button 2 (C# with event.stopPropagation)</button>
    </div>

    <div>
        <button onclick="@((e) => Alert("clicked!"))">Button 3 (C# without event.stopPropagation)</button>
    </div>
</div>

@functions
{
    private void Alert(string message)
    {
        RegisteredFunction.Invoke<int>("alert", message);
    }
}

@Andrzej-W
Copy link

@SteveSandersonMS your answer to this question:

Is there a way in onclick (I guess using UIMouseEventArgs?) to stop the click from propagating?

was:

Not presently. We will add something along these lines if it becomes a common request though.

I think that it is already a common request (problem): #5544, #15908, #15932, #15965, #16045, aspnet/Blazor#715. People must be able to stop the propagation of events. There are situations in which events should always propagate (default behaviour) or always not propagate. Maybe this can be solved by adding a new attribute like on<event-name>-stop-propagation. Sometimes we want to make decision in event handler. You said that it is difficult. I have no experience in JS and worker threads but maybe it is easier to do conceptually something similar to exception handling:

  • if there is event handler in C# code, event will not propagate
  • if the programmer decides he wants the event to propagate he must call the function PropagateEvent() - similarly to (re)throw in exception handling code

@SteveSandersonMS
Copy link
Member

OK, yes, that is pretty clear. We will come up with some way of doing it. Like you suggest, it will probably be a variation on the event-registration syntax, e.g.,

<button onclick=@MyHandler onclick-stop-propagation="true" />

Filed #5545

@SteveSandersonMS
Copy link
Member

BTW, about this:

Sometimes we want to make decision in event handler.

Could you describe scenarios for this? It's a nontrivial bit of functionality to implement, so we'd need more info to know how much to prioritise it.

@RemiBou
Copy link
Contributor

RemiBou commented Jul 3, 2018

I think this would also resolve #15908

@Andrzej-W
Copy link

Andrzej-W commented Jul 6, 2018

Sometimes we want to make decision in event handler.

Could you describe scenarios for this? It's a nontrivial bit of functionality to implement, so we'd need more info to know how much to prioritise it.

@SteveSandersonMS
As we all know HTML input element is primitive and there are a lot of component vendors who create specialized components to enter dates, times, monetary values, etc. There are also much more powerful components like spreadsheets, schedulers, report designers, etc. In those components correct keyboard and mouse handling is very important. Here is a very simple and incomplete example - you can use Plus and Minus keys to increment/decrement numeric value:

<input bind="@TestValue" onkeydown="@KeyHandler" />
int TestValue { get; set; }
void KeyHandler(UIKeyboardEventArgs ev)
{
    if (ev.Key.CompareTo("+") == 0)
        TestValue++;
    else if (ev.Key.CompareTo("-") == 0)
        TestValue--;
}

Press Plus key and you will see 1+ in the input box. Press Minus key and you will see 0-. You can change onkeydown to onkeyup but it is even worse. Try to press and hold down Plus key. You will see a lot of + characters in the input box. Release the key and notice that value is incremented only once.

I have to do something like this to handle keyboard events correctly:

// By default events are always passed to element
void KeyHandler(UIKeyboardEventArgs ev)
{
    if (ev.Key.CompareTo("+") == 0)
    {
        TestValue++;
        ev.Handled = true;
    }
    else if (ev.Key.CompareTo("-") == 0)
    {
        TestValue--;
        ev.Handled = true;
    }
}

or

// By default events aren't passed to elements if C# event handler is attached
// (preventDefault() is called in JS before event is send to C# code)
void KeyHandler(UIKeyboardEventArgs ev)
{
    if (ev.Key.CompareTo("+") == 0)
        TestValue++;
    else if (ev.Key.CompareTo("-") == 0)
        TestValue--;
    else
        DispatchEvent(ev);
}

Maybe the second solution is easier to implement. I have no experience in JavaScript programming but maybe this will be useful:
https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Creating_and_triggering_events
and especially these examples:
https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Creating_and_triggering_events#Triggering_built-in_events
https://developer.mozilla.org/samples/domref/dispatchEvent.html

New DispatchEvent function should have some additional parameters, for example bool canBubble and we should have different DispatchEvent functions for different event types.

@shawty
Copy link

shawty commented Nov 29, 2018

@SteveSandersonMS Further example for you steve, this one causes more problems too, beacuse of the frequency of the events, it actually trashes the GC fairly quickly.

<div id="navbar">
  <ul>
    <li class="navbarItem" onmouseover="@overHandler">
      <a href="#">Link 1</a>
      <div class="subMenu sub1 @(menuIsOpen ? "open" : "")">
        <ul>
          <li><a href="#">Link 1a</a></li>
          <li><a href="#">Link 1b</a></li>
          <li><a href="#">Link 1c</a></li>
          <li><a href="#">Link 1d</a></li>
        </ul>
      </div>
    </li>
    <li class="navbarItem"><a href="#">Link 2</a></li>
    <li class="navbarItem"><a href="#">Link 3</a></li>
    <li class="navbarItem"><a href="#">Link 4</a></li>
    <li class="navbarItem"><a href="#">Link 5</a></li>
  </ul>
</div>

And functions:

bool menuIsOpen;
int cnt = 1;

private void overHandler(){
  Console.WriteLine(cnt);
  cnt++;
  if (menuIsOpen) return; // We only want it called once.
  menuIsOpen = true;
}

The idea is that on a mouseOver on the parent, the class on the sub menu is toggled to open the menu, and if the menu is open the handler exits immediately.

The reality is, that handler gets fired for onmouse over on every element inside the li tag, and the li itself, which menas you move on the menu, the pop opens, then you move down to your menu option, and in the process generate maybe 10 or even 20 events all in rapid succession, which leads to GC nursery full error in the run time.

@xclud
Copy link

xclud commented Nov 29, 2018

OK, yes, that is pretty clear. We will come up with some way of doing it. Like you suggest, it will probably be a variation on the event-registration syntax, e.g.,

<button onclick=@MyHandler onclick-stop-propagation="true" />

Filed #5545

Learning from years of game development (DirectX and OpenGL) and custom UI engines, I believe having onclick-stop-propagation="true/false" is not a good idea. In many cases it happens i want to propagate/stop the event based on some states/condition or even by asking the server (thanks to the async thing), if this event should be dispatched or not.

With above solution, it turns out i have to bind onclick-stop-propagation to some property and evaluate the property before the button is clicked (I must know when user wants to click!! and start to ask the server in advance!!). Even evaluating such property inside onclick function wont help because resolving a thread/promise/task takes some time and as a result the event is already propagated.

I believe having the same mechanism like what HTML and WinForms and WPF do are would make more sense and they look more intuitive. More or less the all do something similar to e.preventDefault().

@Andrzej-W
Copy link

@shawty GC nursery full is not an error.
From https://www.mono-project.com/news/2017/05/17/concurrent-gc-news/

In Mono, the Nursery is the name that we give to the youngest heap generation. When we talk about performing a minor collection, we talk about performing a collection that is limited to the nursery.

We are all waiting for better event system in Blazor.

@shawty
Copy link

shawty commented Nov 29, 2018

@Andrzej-W I don't know the internals of the Blazor GC Andrezej, so when I see messages like that appearing in my browser console, it looks like an error (To me and the many others who see it).

All I was doing was adding another common(ish) example to the list so the guys have more to work with in order to help them achive the goal.

@Andrzej-W
Copy link

@shawty the first time I saw this message a few months ago I also thought that it was an error - so don't worry and be happy that we can finally write web applications in C# 😃

@michaelvolz
Copy link

michaelvolz commented Dec 2, 2018

@xclud I totally agree. I would rather have my code decide to prevent or not than having extra noise with additional attributes in my tags.

@shawty
Copy link

shawty commented Dec 4, 2018

@Andrzej-W don't worry dude, it's all good. My reply was meant to sound the way it did, I'd had some clients winding me up erlier that day, so I wasn't in a perticularly patient or forgiving mood :-)

Lessons to be learned, never write angry when on git-hub.

Blazor rocks, no two ways about that.

@mkArtakMSFT mkArtakMSFT transferred this issue from dotnet/blazor Oct 27, 2019
@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Oct 27, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

No branches or pull requests

9 participants