Question: MouseEvent::mouseWasClicked()

General JUCE discussion
1

it seems that this method returns true even if you don't click the mouse:

void MyComponent::mouseOver( const MouseEvent &event ) {
  if( event.mouseWasClicked() ) {
    std::cout << "mouseOver: CLICKED\n";
  }
}

looks like the problem might be here:

bool MouseEvent::mouseWasClicked() const noexcept { return wasMovedSinceMouseDown == 0; }

here is line: 2562 in juce_Component, it looks like wasMovedSinceMouseDown is initialized with 'false'

const MouseEvent me (source, relativePos, source.getCurrentModifiers(), MouseInputSource::invalidPressure,
                             this, this, time, relativePos, time, 0, false);
mouseMove (me);

here's how wasMovedSinceMouseDown is initialized in the mouseEvent constructor:

wasMovedSinceMouseDown ((uint8) (mouseWasDragged ? 1 : 0))

mouseWasClicked() returns true, because wasMovedSinceMouseDown is initialized to uint8 '\0'

 

I'm not sure if this is a bug or not, but when i'm mousing over a window without clicking on the window, I am not expecting "mouseWasClicked()" to return true.  

 

It seems like you should add a member to the mouseEvent class that specifically stores if the event was created out of a mouse-click, no? 

 

Here is a possible fix:

bool MouseEvent::mouseWasClicked() const noexcept {
  return (numberOfClicks != 0);
}

 

2

Like the docs for that method say, it's designed for use in your mouseUp or mouseDrag callback. It's meaningless to call it anywhere else.

3

ok, but either way that method doesn't actually check if the mouse was clicked.  it checks it if was dragged.   

method title does not reflect method purpose.   

you should either change the method name to reflect what it does (might I suggest bool mouseMovedSinceClick() ) or make it do what it says it should do in the name, so we can use this with ALL mouseEvent types, not just MouseUp and MouseDrag. 

bool MouseEvent::mouseWasClicked() const noexcept { return (numberOfClicks != 0); }

In my particular scenario, I have a helper function that both my MouseDown and MouseMove methods pass their event to, which draws pitch accidentals on the screen.  if the helper function is called from MouseMove, then i add a few extra lines of logic compared to mouseDown.  My workaround was 

if( event.getNumberOfClicks == 0 )

to differentiate between the two callers

4

I agree, this method should be renamed

5

cool.  bump this topic so it gets noticed by Jules/Timur/etc.   it's a real simple change and will make the code more understandable

6

Breaking everybody's existing code by renaiming a method isn't a simple change!

And in context, I really don't think the name is so bad. Inside a mouseUp method, to want to know "was the mouse clicked" is a sensible way to phrase the question, IMHO.

7

Still, it could be marked as deprecated in favour of a more apt method name.

I second the idea that a method named like that doesn't convey the concept that it doesn't actually check for clicks, but instead tells you if the mouse has moved significantly.

8

Maybe we should add that to the documentation then.

+1 to Jules that this is not a method we can rename without breaking a lot of code. And I'd reckon we really don't want to do here... the official deprecation route is reserved for more serious cases where methods actually had a broken API, or need to be fundamentally redesigned to add a new feature, etc...

Please don't get me wrong, I am a HUGE fan of proper method names that precisely convey the method's purpose.

9

lol what part of 

bool MouseEvent::mouseWasClicked() const noexcept { return wasMovedSinceMouseDown == 0; }

makes you think this method title is appropriate, considering what the method actually does? 

10

+1