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

Let .hasEvent work with array iteration methods. #1183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryanve
Copy link
Contributor

@ryanve ryanve commented Jan 8, 2014

This simple change lets .hasEvent(event, element?) work with methods like array.every or array.filter. It checks if element is a number and if so it uses this as the element.

['drag', 'dragend'].every(Modernizr.hasEvent) // uses 'div' in strict mode, `window` in non-strict
['drag', 'dragend'].every(Modernizr.hasEvent, 'img') // creates element on each iteration
['drag', 'dragend'].every(Modernizr.hasEvent, document.createElement('img')) // reuses element

Explicitly supplying the thisArg avoids the strict mode difference.

@stucox
Copy link
Member

stucox commented Feb 23, 2014

Hi Ryan, sorry it’s taken a while to get to this – looks good.

I don’t get why there’s a difference between strict mode / non-strict mode though? My knowledge of the details of strict mode is embarrassingly lacking though, tbh.

@ryanve
Copy link
Contributor Author

ryanve commented Feb 24, 2014

I accounted for both modes in case 'use strict'; was ever added here or in an outer closure.

strict mode

  • No boxing occurs. thisArg values resolve exactly. this defaults to undefined

unstrict mode

  • this resolves to window when thisArg is undefined or null
  • Primitive thisArg values get boxed into the "object" type. .valueOf() unboxes them (if primitive) but throws if used on undefined|null|Object.create(null)

if ( !element || typeof element === 'string' ) {
element = createElement(element || 'div');
}
element = typeof element === 'number' ? this && this.valueOf() : element;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider these variants:

  1. 100% consistent between strict and unstrict mode:
    if ( typeof element === 'number' ) element = null == this ? window : this.valueOf();
  2. Throws error in strict mode if this == null but is consistent between modes for other cases:
    if ( typeof element === 'number' ) element = this.valueOf();
  3. Needs another check to make tagnames work in unstrict mode and the default varies by mode:
    if ( typeof element === 'number' ) element = this;
    if ( element instanceof String ) element = '' + element;

@ryanve
Copy link
Contributor Author

ryanve commented Feb 24, 2014

Consider the docs, and keeping them simple. .hasEvent can be used with array methods by passing an element or tagname as the thisArg. Then show examples. Questions to answer are:

  • Should we add the syntax at all? (It's not needed, but it helps write terse code)
  • Should it support tagnames? (If no then the first line of variant 3 would suffice)
  • Should it worry about strict mode? (I like variant 1 if yes, and my original or 2 if no)

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.

None yet

2 participants