-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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
Add .bare event modifier, fix #5976 #5977
Conversation
If decided to be merged - should be documented |
@@ -3660,7 +3660,8 @@ var modifierCode = { | |||
meta: genGuard("!$event.metaKey"), | |||
left: genGuard("'button' in $event && $event.button !== 0"), | |||
middle: genGuard("'button' in $event && $event.button !== 1"), | |||
right: genGuard("'button' in $event && $event.button !== 2") | |||
right: genGuard("'button' in $event && $event.button !== 2"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove everything from packages
? Those files are generated 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, didn't know
Add a new event modifier `.plain` to check if event is "plain" (no shift/ctrl/meta/alt key is pressed) vuejs#5976
@posva, removed the |
@nickmessing I added this PR should make it easier for new contributors. I'll see what the team thinks about this. #5984 |
@blake-newman, I knew about |
@nickmessing yeah it's fairly easy to miss, and sometimes I add dist files by accident and it is annoying amending commits |
This doesn't seem to work if I do
Or is not the purpose of this PR to introduce this? |
@asumaran |
@posva Thanks, I misunderstood the real issue 👍 |
Is |
My English is not good🙂, and I can only think of |
Even though I approved the change, @nickmessing made it clear that he isn't convinced either by the name either. We should think about it and we'll probably have a better name in a few days on top of my head I can think about |
"bare" maybe? A bare event seems to make sense meaning nothing additional |
@posva, actually, I really like |
It would be generally useful to have a Alternatively it would be ok if one could simple re-emit the event and define multiple handlers like such:
This code actually works, but logs an error of course (because $event param should be a string but then it doesn't work). If this worked the first |
the |
@posva, I actually like the idea of |
@posva what about a way to at least declare two handlers as in my example without causing exceptions in order to re-emit non-handled cases. It will be a question of specificity of course. Or is there another way to cleanly re-emit bubbling native events (the fact that component events don't bubble is not ideal anyway, but that is another topic) |
@nickmessing It could be added but I personally feel like it's too much because those modifiers generate js code atm, which makes things different from the order. IMO, it's better to write the actual javascript. @matthiasg I'd add one single event handler and do the checks there |
@posva sure we could do that, but in our architecture, we try to get most basic stuff done in the html or at the very least without specifics of structures like the html/dom eventing system. Vue and angular before are interesting to us, because they do not try to force people to use javascript for everything and our customers can follow at their own pace. as for the specifics of ordering, a single prefixed |
@matthiasg, |
/ping @posva, @kazupon, @blake-newman, @Kingwl Any ideas what we do with this? Is |
I think @hworld made a better proposition with bare |
|
@nickmessing How about a new event modifier |
@joshfisk Then I guess |
@dsonet Yeah, ".only" works better. It is easy to understand, regardless of the order. |
@dsonet, @joshfisk, that would mean a pretty big change in the way modifiers are implemented, the easy way would be to add |
@nickmessing It could initially support normal keys, but be expanded to support modifiers in the future. A common use is showing valid shortcut keys when you hold a modifier. This is commonly done with the Alt key, and menus. |
@joshfisk, I will try to find an easy way of making |
@nickmessing Here are two example problems:
|
@joshfisk, unfortunately that cannot be achieved since in scenario of both Y and N pressed two keyup event happend - one for |
@nickmessing I see what you mean. I prefer two of the suggested changes working together. What is the reason 'stop', and 'prevent' are in the same lookup table as the modifier filters? |
@nickmessing https://gist.github.com/JoshFisk/f407ac7c74c7123ae4594249cbcc3560 |
@joshfisk, yeah, |
Allow limiting the event to the exact system modifiers specified. close vuejs#5976
Allow limiting the event to the exact system modifiers specified. close vuejs#5976
Add a new event modifier
.bare
to check if event is "bare" (no shift/ctrl/meta/alt key ispressed)
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch for v2.x (or to a previous version branch), not themaster
branchfix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
Discussion: #5976