supermedium/superframe

event-set cannot set attributes on components with a dash in their name.

Opened this issue · 6 comments

The docs for event-set state:

The event-set component also works with components that have multiple properties using A-Frame component dot syntax (i.e., ${componentName}.${propertyName})

https://aframe.io/docs/1.2.0/introduction/interactions-and-controllers.html#handling-events-with-event-set-component

This seems to work fine - except in the case where a component has a dash in it's name, then event-set cannot set the property, and instead unpredictable results can follow.

For an example of the problem see this glitch:
https://prickle-boulder-worm.glitch.me/

The code for the two rectangles is identical except that one uses component "my-position" the other uses (identical) component "myposition". When Enter is pressed the 2nd one moves position as expected. The first simply disappears!

Looking at what's gong on in event-set in the debugger, it appears that by the time the component name "my-position" arrives in the event-set component, A-Frame has already transformed it into "myPosition". The later attempt to look this up in the components object seems to fail.

"animation" is an example of another A-Frame component which can set attributes on individual components. This uses a dedicated "property" attribute for the property to be set, and seems to work fine with all component names, whether or not they have dashes.

See this glitch for an illustration similar to the one above:
https://spiky-caring-bicycle.glitch.me/

So one solution might be to update the event-set interface to use an explicit "property" attribute to allow the specification of the property to be set.

I suspect this new interface could be supported in parallel with the old interface, to enable back-compatibility. I'd be happy to make this change as a PR if it would be accepted.

There may be other solutions to the problem, but it seems they would require some changes to the A-Frame infrastructure itself...

In the meantime, the workaround I have been using is, for any component with dashes in the name, where I want to use event-set, I create another wrapper component without the dashes, and I use that instead...

Not sure I understand this statement:

Looking at what's gong on in event-set in the debugger, it appears that by the time the component name "my-position" arrives in the event-set component, A-Frame has already transformed it into "myPosition". The later attempt to look this up in the components object seems to fail.

A-Frame doesn't transform component names as far as I can tell. Maybe something unintended.

I updated the glitch to use the non-minified version of event-set for easy debugging.

Set a breakpoint on update() and refreshed the page.

For the second object, this.data looks like this...

image

Note that the field in this.data is "myPosition". Whereas the supplied parameter in the HTML was "my-position".

Now, set a breakpoint in the event handler, and trigger the event. First one (myposition) is fine. In the second one, we see that event-set is trying to set an attribute on the "myPosition" component, which does not exist (it's called "my-position").

image

(as we see in the final screenshot, where I have looked at "targetEl.components" in the watch screen at the same point in the debugger)

image

Does that make sense?

The glitch I am referring to is this one: https://prickle-boulder-worm.glitch.me/
(which now uses non-minified event-set)

The A-Frame style like parser used in the component forces camel casing for property names. A-Frame doesn't allow hyphens in component property names. In the case of this component the property names correspond to component names that allow dash symbols.

A solution could be to not use the A-Frame built-in parser and replace it with something that honors the hyphens.

@dmarcos - did you see my original suggestion to follow the pattern used by the animation component, and have a "property" attribute that contains the name of the component & property to set?

We could enable this alongside the existing interface as an option to use for cases where the current interface doesn't work.

If you think you'd accept that solution as a PR, I'm happy to implement it.

It's possible that the current behavior of AFRAME.utils.styleParser.stringify is not very much in line with the rule in this alert

image