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

Position the bubble help to the lower right of the mouse #1353

Merged
merged 1 commit into from Oct 10, 2022

Conversation

bmatherly
Copy link
Member

@ddennedy
Copy link
Member

ddennedy commented Oct 9, 2022

I do not agree that it addresses the problem or request. Maybe it is better positioned but I do not know yet and because it changes so many contexts that need evaluation for somewhat arbitrary change that is maybe not worth the time. What I do notice is that v22.09's changes adversely affects the vertical position. That is what needs to be addressed.

@bmatherly
Copy link
Member Author

What I do notice is that v22.09's changes adversely affects the vertical position. That is what needs to be addressed.

Maybe that is what I was noticing. I did not go back to previous versions to compare if this was a regression. But in 22.09, the bubble would be arbitrarily lower than the mouse pointer - sometimes by a whole clip. In the case of the timeline ruler, the bubble would be covering the "good" part of the audio waveform of clips on the track when the mouse is over the ruler.

Now that you mention it, this vertical offset could have been caused by removing the toolbar from QML.

Regardless, this change puts the bubble help consistently and predictably near the mouse pointer. So I think it is generally an improvement - in addition to resolving the presumed regression. I did test it in all the contexts where "show" is called. But I may not have tested every possible variation of having adjacent clips, etc.

Comment on lines 519 to 520
bubbleHelp.x = point.x + 12;
bubbleHelp.y = point.y + 12;
Copy link
Member

Choose a reason for hiding this comment

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

These 12 values are dependent on the mouse cursor size, which I doubt there is a way to get in QML, but some people will run with a larger cursor and then the bubble help appears underneath a little, which is not a big deal but worth taking into consideration instead of being tight like this.

Copy link
Member

Choose a reason for hiding this comment

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

            bubbleHelp.x = point.x + 20;
            bubbleHelp.y = Math.max(point.y - 20, 0);

works better for me to prevent obscuring waveforms and dealing with a larger cursor size.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking of it as a tooltip and tried to emulate that visual look/position. And in fact, I was even thinking about re-implementing the bubble help using QToolTip. But that is a bigger change, and I like the "bubble" look that we have.

Tool tips typically appear in the lower right - presumably because we read top-to-bottom/left-to-right:
image
Also, tool tips often have logic to move the box around to avoid being clipped off the screen:
image
I did not bother to write extra logic to avoid clipping the bubble help box

In this specific case, I think your suggested offsets do work better in the timeline. So I made that change. But I am open minded about it and we can tweak them. I am also willing to re-implement the bubble help as a QToolTip - but then we will have less control over the position. However, that would increase consistency with the rest of the Shotcut UI.

Copy link
Member

Choose a reason for hiding this comment

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

It was not intended to be a tooltip. I don’t want to reimplement many things in the GUI in the near- or mid-term.

@ddennedy ddennedy merged commit ce9f65a into mltframework:master Oct 10, 2022
@bmatherly bmatherly deleted the bubble_position branch November 28, 2022 00:49
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.

None yet

2 participants