html-next / ember-hammertime

TouchAction (aka "fastclick") Support for Ember Applications
MIT License
56 stars 23 forks source link

Prevent the AST Walker to add the touch-action styles multiple times to DOM Element within a loop #60

Open mkamneng opened 6 years ago

mkamneng commented 6 years ago

The ast walker is adding the touch-action styles multiple times to each dom element withing a loop as described in this example:

//JS file
import Component from '@ember/component';
export default Component.extend({
    actions: {
        jumpTo() {
            //
        }
    },

    init() {
        this._super(...arguments);
        this.set('data', [1,2,3]);
    }
});

// HBS file
{{#each data as |item|}}
    <div class="jumpable-{{item}}" onclick={{action 'jumpTo'}}>Hammer Time {{item}}</div>
{{/each}}

// DOM result 

<div style="touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;" class="jumpable-1">Hammer Time 1</div>
<div style="touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;" class="jumpable-2">Hammer Time 2</div>
<div style="touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;" class="jumpable-3">Hammer Time 3</div>

The solution could be as simple as a check in the walker if the node has already a touch action properties, if I'm not missing something:

TouchActionSupport.prototype.transform = function TouchActionSupport_transform(ast) {
  var pluginContext = this;
  var walker = new pluginContext.syntax.Walker();

  walker.visit(ast, function(node) {
    if (pluginContext.validate(node)) {
      var style = elementAttribute(node, 'style');
      if (!style) {
        style = {
          type: 'AttrNode',
          name: 'style',
          value: { type: 'TextNode', chars: '' }
        };
        node.attributes.push(style);
      }
      // insert the touch action properties only if not present
      if(style.value.chars.indexOf(touchActionProperties)===-1) {
          style.value.chars += touchActionProperties;
      }
    }
  });

  return ast;
};

I can open an PR with a test if that can help!

Thanks

RobbieTheWagner commented 6 years ago

A PR would be great, thanks!

eriktrom commented 6 years ago

added once by the ast parser, once by the touch-action mixin, fyi

i'll maybe take a look but that's the problem fwiw

eriktrom commented 6 years ago

remove the end of this line (after the ||) or remove the onclick from your code, to clarify why

https://github.com/html-next/ember-hammertime/blob/master/addon/mixins/touch-action.js#L12