dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.15k stars 9.92k forks source link

@onevent:preventDefault does not trigger when event is not already registered #18449

Open poke opened 4 years ago

poke commented 4 years ago

When using the new @onevent:preventDefault directive to prevent the default event behavior for events, I noticed that in order for this to work, there needs to be a @onevent handler registered anywhere on the page.

Apparently, server-side Blazor registers every event just once on the document instead of on individual elements. So e.g. @onclick="…" causes the click event to be subscribed on the document. Just using @onclick:preventDefault on the page does not appear to register this global event handler on document, so the preventDefault() call is also never made in the handler.

My use case for this is managing focus with buttons: When you click a button, then the button stays focused after clicking it, causing the focused style to be applied. In order to avoid this, you can subscribe to the mousedown event in JavaScript and just prevent the default behavior:

<button id="test">Test</button>
<script>
    var button = document.getElementById('test')
    button.addEventListener('click', () => console.log('Clicked'));
    button.addEventListener('mousedown', (evt) => evt.preventDefault());
</script>

Doing the same in Blazor would look like this:

<button @onclick="Click" @onmousedown:preventDefault>Test</button>

@code {
    void Click() => Console.WriteLine("Click");
}

This however does not work if there isn’t also some mousedown handler registered somewhere. As soon as you add one literally anywhere, it works:

<button @onclick="Click" @onmousedown:preventDefault>Test</button>
<div @onmousedown="() => { }"></div>

@code {
    void Click() => Console.WriteLine("Click");
}

This is using server-side Blazor on ASP.NET Core 3.1.

javiercn commented 4 years ago

@poke thanks for contacting us.

@poke what you are mentioning makes sense.

@SteveSandersonMS do you have any thoughts here?

javiercn commented 4 years ago

@poke on a second look, I think the current behavior is actually fine.

I think that as a rule, for simplicity we want to keep the behavior where something:modifier attributes to only apply if you also have a something applied, like in this case mousedown.

The reason is for simplicity and consistency within the language. If we were to do so, every time we introduce a new modifier we will have to think if it makes sense to apply it in isolation or not, and that will make the rule for modifiers very complicated (as it would depend on the modifier being applied).

The alternative, even though it requires a bit more code is simpler to understand and explain. If you want to apply some modifier something:modifier you also need to apply something, even if that doesn't cause any action.

/cc: @SteveSandersonMS in case he has other thoughts.

poke commented 4 years ago

where something:modifier attributes to only apply if you also have a something applied

But that’s not what is the case here. Please note that in my example, I use @onmousedown:preventDefault on a button element while I have to apply @onmousedown on literally any other element.

Your statement makes it sound as if you would need to put both on the same element like this:

<button @onclick="Click"
        @onmousedown="() => { }"
        @onmousedown:preventDefault>Test</button>

If that was the case, I would probably accept this (although I would really dislike it). But the thing is that you do not need @onmousedown on the element. You just need it somewhere throughout the whole app to make Blazor’s client side runtime register the event once on the document element.

The fact that this is so disconnected from the @onmousedown:preventDefault makes this very confusing, and definitely not “simpler to understand and explain”. And considering that this is preventDefault it is also very common to put this on events that you will usually not care about, so requiring an empty event handler just to make Blazor realize that the event handler is needed feels very redundant.

I understand the desire to keep the attribute:modifier handling simple but it shouldn’t be too problematic to make @event:modifier also trigger an empty registration of @event.

SteveSandersonMS commented 4 years ago

There's been a bit of confusion here. I think there is a bug to be addressed here.

I think that as a rule, for simplicity we want to keep the behavior where something:modifier attributes to only apply if you also have a something applied, like in this case mousedown.

That's not how this was designed - see #14517:

For each @oneventname, there's also @oneventname:preventDefault and @oneventname:stopPropagation. These can be used either alongside @oneventname or independently of it.

I do agree there are valid use cases for using preventDefault/stopPropagation independently of handlers for the same event on the same element; that's something we always intended to support and in fact do support today. The bug is when you happen not to have any handlers for a given event type anywhere in the document.

poke commented 4 years ago

@SteveSandersonMS Thanks a lot for the clarification :)

ghost commented 4 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 4 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ysmoradi commented 2 years ago

Facing the same issue )":

msynk commented 2 years ago

any updates? It's been 2 years + 1 day exactly now 😀

danroth27 commented 2 years ago

@SteveSandersonMS @javiercn Is this something we think we can still address for .NET 7? Or is the risk too high at this point?

SteveSandersonMS commented 2 years ago

@danroth27 Let's discuss it with the team. I don't think the risk is particularly high though if we implemented a fix we'd have a more precise sense of how disruptive the change is.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

artemiusgreat commented 1 year ago

This is still the issue. I was trying to make sure that event is not propagated outside of some element, e.g. Google Maps, using the mouse scroll user can scale the map, but the page that contains this map should not scroll. In case of Blazor, everything is scrollable, no matter how I try to stop the propagation outside of the element.

<div class="canvas-chart"
    @onwheel="OnWheelAction"
    @onmousewheel="OnWheelAction"
    @onmousemove="OnMouseMoveAction"
    @onmouseout="OnMouseLeaveAction"
    @onmousedown="OnMouseDownAction"

    @ontouchmove:preventDefault
    @ontouchcancel:preventDefault
    @onwheel:preventDefault
    @onmousewheel:preventDefault
    @onmousemove:preventDefault
    @onmouseout:preventDefault
    @onmousedown:preventDefault

    @ontouchcancel:stopPropagation
    @ontouchmove:stopPropagation
    @onwheel:stopPropagation
    @onmousewheel:stopPropagation
    @onmousemove:stopPropagation
    @onmouseout:stopPropagation
    @onmousedown:stopPropagation>
</div>

In JS, simple preventDefault does the trick. http://jsfiddle.net/eXQf3/371/

ghost commented 11 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 9 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

wstaelens commented 5 months ago

What is the status of this?

image

msynk commented 5 months ago

the new record: +4 years 😲

still no update for this issue?

Yomodo commented 5 months ago

the new record: +4 years

There's no need for this.

tomer-asensus commented 5 months ago

Mudblazor has a working solution for that:

https://mudblazor.com/components/swipearea#prevent-default-browser-behavior

ScarletKuro commented 1 month ago

Mudblazor has a working solution for that

It's an ugly workaround. We really hope that this issue will be addressed together with https://github.com/dotnet/aspnetcore/issues/24932