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

sq #238

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

androidStern
Copy link

Draft for #230

Things this does:

  1. changes mouseEnter and mouseExit to act like browsers
  2. responds to drag events at a top level component and turns them into mouseEnter and mouseExit events when necessary

Things this doesn't do:

  1. handle multitouch

BIG DISCLAIMER: I'm not a C++ guy. I'm sure the final PR will have much better code. Theres probably bugs. This is just to facilitate a design discussion.

@@ -103,6 +103,7 @@ namespace reactjuce
/** Invokes an "exported" native method on the View instance */
juce::var invokeMethod(const juce::String &method, const juce::var::NativeFunctionArgs &args);

void checkMouseEnter(const juce::MouseEvent& e);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should really be called something like checkMouseEnterAndExit.

}

void View::mouseExit (const juce::MouseEvent& e)
{
dispatchViewEvent("onMouseLeave", detail::makeViewEventObject(e, *this));
if (!reallyContains(e.getPosition(), true))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reallyContains returns true even if the mouse is over one of this components children.

@@ -63,6 +63,29 @@ namespace reactjuce
bool keyPressed(const juce::KeyPress& key) override;
#endif

juce::Component* componentUnderMouse;

void mouseDrag(const juce::MouseEvent &e) override {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used to translate mouseDrags into enter and exit events

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm interesting, won't ReactAppRoot not get this callback if the mouse is dragged over a child component? https://docs.juce.com/master/classComponent.html#aff01e4d339bda474a66984b709044b63

Or is AppRoot getting this callback at all times?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see View::mouseDrag I forward child drag events onto ReactAppRoot::mouseDrag.

but yes, its pretty ugly. Another way to handle it would be making ReactAppRoot a mouse listener for all its children I suppose.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks! Finally coming back around to this, sorry for the delay.

So, reading this through again with a better understanding, I actually think this is quite like the way juce implements its own drag and drop patterns, except here we're sort of re-implementing it with ReactAppRoot == DragAndDropContainer and View == DragAndDropTarget. (See https://github.com/juce-framework/JUCE/blob/b8206e3604ebaca64779bf19f1613c373b9adf4f/modules/juce_gui_basics/mouse/juce_DragAndDropContainer.cpp)

This has me thinking then, especially in the context of the approach we laid out in the drag and drop issue (see #60 (comment)), that perhaps we should just merge this goal into that work. The approach laid out in that issue is basically 1 step away from solving this problem here, and we can nudge the dragstart/dragmove/dragend callbacks that each View will receive (because its a DragAndDropTarget) to correctly send mouseenter/mouseleave callbacks as well.

What do you think?

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

Successfully merging this pull request may close these issues.

2 participants