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

Overview: clear pickup position display when opening cue menu #13693

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Sep 24, 2024

IMO it only adds noise, all relevant info like the cue position is displayed in the cue menu.

@github-actions github-actions bot added the ui label Sep 24, 2024
@ronso0 ronso0 marked this pull request as ready for review September 24, 2024 22:45
Comment on lines 597 to 598
QEvent* ev = new QEvent(QEvent::HoverLeave);
leaveEvent(ev);
Copy link
Member

Choose a reason for hiding this comment

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

ev is leaked. This should do the trick:

Suggested change
QEvent* ev = new QEvent(QEvent::HoverLeave);
leaveEvent(ev);
QEvent ev = QEvent(QEvent::HoverLeave);
leaveEvent(&ev);

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you.
I definitly have to do some homework wrt pointers.

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

Copy link
Member

Choose a reason for hiding this comment

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

when in doubt, never write "new" :).

@ronso0 ronso0 force-pushed the overview-clear-pickup-pos branch from 1e217b1 to 3d8627b Compare September 26, 2024 07:11
@ronso0 ronso0 force-pushed the overview-clear-pickup-pos branch from 3d8627b to af6057b Compare October 5, 2024 13:51
@@ -592,6 +592,10 @@ void WOverview::mousePressEvent(QMouseEvent* e) {
m_pCurrentTrack->removeCue(pHoveredCue);
return;
} else {
// Clear the pickup position display.
// ev could actually be any event type, it's not used anyway.
Copy link
Member

Choose a reason for hiding this comment

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

if it's not used can we pass nullptr?

Copy link
Member Author

Choose a reason for hiding this comment

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

let's find out..

Copy link
Member Author

Choose a reason for hiding this comment

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

works, ready for merge!

@ronso0 ronso0 force-pushed the overview-clear-pickup-pos branch from af6057b to 95536c0 Compare December 18, 2024 19:45
@ywwg ywwg merged commit 4d9e709 into mixxxdj:2.5 Dec 19, 2024
13 checks passed
@ronso0 ronso0 deleted the overview-clear-pickup-pos branch December 19, 2024 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants