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

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

Open
diarmidmackenzie opened this issue Apr 3, 2021 · 6 comments

Comments

@diarmidmackenzie
Copy link
Contributor

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...

@dmarcos
Copy link
Member

dmarcos commented Apr 5, 2021

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.

@diarmidmackenzie
Copy link
Contributor Author

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?

@diarmidmackenzie
Copy link
Contributor Author

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

@dmarcos
Copy link
Member

dmarcos commented Apr 6, 2021

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.

@diarmidmackenzie
Copy link
Contributor Author

@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.

@isaac-pj
Copy link

isaac-pj commented May 7, 2023

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

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

No branches or pull requests

3 participants