Skip to content

Conversation

@Kingwl
Copy link
Member

@Kingwl Kingwl commented Mar 8, 2017

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

@Kingwl Kingwl changed the title support v-on passive modifier [!WIP] support v-on passive modifier Mar 8, 2017
window.addEventListener('test-passive', null, opts)
} catch (e) {}
}
export const supportsPassive = _supportsPassive
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES export bindings track changes, we can just do

export let supportsPassive = false
// feature detect...

Copy link
Member Author

@Kingwl Kingwl Mar 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done thanks

expect(spyDown.calls.count()).toBe(1)
})

it('should support passive', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case should only run when the test browser supports passive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@yyx990803
Copy link
Member

Looking good! I'll merge this when we do the next minor release.

@Kingwl Kingwl changed the title [!WIP] support v-on passive modifier support v-on passive modifier Mar 8, 2017
@Kingwl
Copy link
Member Author

Kingwl commented Mar 8, 2017

#5118

@HerringtonDarkholme
Copy link
Member

Just a suggestion, since Vue also support prevent modifier https://vuejs.org/v2/guide/events.html#Event-Modifiers. It would even better if compiler can detect and warn the mutual exclusive prevent and passive.

@Kingwl
Copy link
Member Author

Kingwl commented Mar 9, 2017

ping
@HerringtonDarkholme @yyx990803
review need

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, what about passive and prevent can't be used together. Passive handler can't prevent default event.

It uses simpler word and explains the reason.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

// warn prevent and passive modifier
if (process.env.NODE_ENV !== 'production' && modifiers && modifiers.prevent && modifiers.passive) {
warn(
'passive and prevent can\t be used together. ' +

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing literal (') in can\t

expect(vm.$refs.normal.checked).toBe(false)
expect(vm.$refs.passive.checked).toBe(true)
expect(vm.$refs.exclusive.checked).toBe(true)
expect('passive and prevent can\t be used together. Passive handler can\'t prevent default event.').toHaveBeenWarned()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs minor fix for above suggested text change.

@yyx990803 yyx990803 merged commit beee7d8 into vuejs:dev Apr 5, 2017
@Kingwl Kingwl deleted the dev-passive branch April 6, 2017 02:26
awamwang pushed a commit to awamwang/vue that referenced this pull request Jun 15, 2017
* support v-on passive modifier

* fix supportsPassive and run unit when the test browser supports

* add mutual exclusive warning

* Fix typo

* Fix typo

* Remove extra line - CS fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants