-
Notifications
You must be signed in to change notification settings - Fork 424
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
event-set cannot set attributes on components with a dash in their name. #296
Comments
Not sure I understand this statement:
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... 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"). (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) Does that make sense? |
The glitch I am referring to is this one: https://prickle-boulder-worm.glitch.me/ |
The A-Frame style like parser used in the component forces camel casing for property names. 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. |
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...
The text was updated successfully, but these errors were encountered: