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

AppendageNode order of change/input firing #153

Closed
jonathanolson opened this issue Jan 12, 2017 · 5 comments
Closed

AppendageNode order of change/input firing #153

jonathanolson opened this issue Jan 12, 2017 · 5 comments

Comments

@jonathanolson
Copy link
Contributor

It would be helpful near here:

    // Due to the variability of input and change event firing across browsers,
    // it is necessary to track if the input event was fired and if not, to
    // handle the change event instead.
    // see: https://wiki.fluidproject.org/pages/viewpage.action?pageId=61767683

to mention that the input event will fire before the change event (in the case that they both fire). The listener for 'change' was in front, so it took a bit to sort out the logic.

Also, this seems like behavior that would be very common, and should be abstracted out (e.g. #149).

@jonathanolson
Copy link
Contributor Author

Code review for #128

@jonathanolson
Copy link
Contributor Author

Also as a note about related code: rotateAppendage() is specific to rotating due to the accessible DOM (that wasn't immediately apparent from the name).

@jessegreenberg
Copy link
Contributor

I updated the documentation for both #153 (comment) and #153 (comment).

@jessegreenberg
Copy link
Contributor

I agree, that much should be abstracted out for all accessible sliders. #149 suggests a mixin pattern for all kinds of accessible components, and this event handling could be handled there.

@jessegreenberg
Copy link
Contributor

Also, this seems like behavior that would be very common, and should be abstracted out (e.g. #149).

Further abstraction of accessible components is going to be tracked in phetsims/scenery#607, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants