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

Combine DragListener and KeyboardDragListener into a single class #1614

Closed
zepumph opened this issue Feb 21, 2024 · 8 comments
Closed

Combine DragListener and KeyboardDragListener into a single class #1614

zepumph opened this issue Feb 21, 2024 · 8 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Feb 21, 2024

While working on phetsims/scenery-phet#849, we found that RichDragListener and RichKeyboardListener are identical, but need to be duplicated because they DragListener and KeyboardDragListener are separate classes. I wonder if we could create a third class that combines them into a single drag listener, and have RichDragListener extend from that. @jessegreenberg @pixelzoom, I couldn't find another issue for this, but please close as a duplicate if I missed it. I know we have discussed this before.

zepumph added a commit to phetsims/sun that referenced this issue Feb 21, 2024
@pixelzoom
Copy link
Contributor

I also thought there was a similar issue, but can't locate it. It's unfortunate that we need to add both a DragListener and a KeyboardDragListener, but I'll be surprised if we find a way around that.

@jessegreenberg
Copy link
Contributor

This is probably a subset of #1238.

I wonder if we could create a third class that combines them into a single drag listener,

Great idea!

@zepumph
Copy link
Member Author

zepumph commented Feb 22, 2024

I also was teasing with the idea of doing it in a different way, where we make KeyboardDragListener a subtype of DragListener. Perhaps calling it FullDragListener. And it may not be too challenging to handle an "off switch" for each type of input for backwards compatibility and complicated cases. No matter, this isn't the kind of work we could just sprinkle into an iteration.

@zepumph
Copy link
Member Author

zepumph commented Apr 17, 2024

Over in phetsims/scenery-phet#849, we need KeyboardDragListener to have an interrupted flag like PressListener. I'll add it here.

zepumph added a commit that referenced this issue Apr 17, 2024
Signed-off-by: Michael Kauzmann <michael.kauzmann@colorado.edu>
@zepumph
Copy link
Member Author

zepumph commented Apr 17, 2024

This was also nice, as it aligned the API for the end callbacks.

pixelzoom pushed a commit that referenced this issue Apr 18, 2024
Signed-off-by: Michael Kauzmann <michael.kauzmann@colorado.edu>
pixelzoom pushed a commit that referenced this issue Apr 18, 2024
Signed-off-by: Michael Kauzmann <michael.kauzmann@colorado.edu>
@jessegreenberg jessegreenberg self-assigned this May 21, 2024
@jessegreenberg
Copy link
Contributor

I am trying out an idea for this in branch richDragListener_scenery_1614. It simply creates both a DragListener and a KeyboardDragListener and forwards the input events to each. Options are set up so that many are shared by the two listeners but you can still provide options to each listener as needed.

jessegreenberg added a commit to phetsims/faradays-electromagnetic-lab that referenced this issue May 28, 2024
jessegreenberg added a commit to phetsims/faradays-electromagnetic-lab that referenced this issue May 28, 2024
jessegreenberg added a commit to phetsims/scenery-phet that referenced this issue May 28, 2024
jessegreenberg added a commit to phetsims/scenery-phet that referenced this issue May 31, 2024
jessegreenberg added a commit to phetsims/scenery-phet that referenced this issue Jun 3, 2024
jessegreenberg added a commit to phetsims/scenery-phet that referenced this issue Jun 4, 2024
@jessegreenberg
Copy link
Contributor

A combined RichDragListener was added to scenery-phet using the approach from #1614 (comment). It is in scenery-phet because it combines the "rich" drag listeners that also include tambo sounds. Ill prune the scenery branch now that we are done with it and transfer this issue to scenery-phet to wrap up.

@jessegreenberg
Copy link
Contributor

Code review will occur in phetsims/scenery-phet#858, closing.

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

3 participants