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

Tooltip issues from Lp1779468 #1752

Merged
merged 8 commits into from
Aug 9, 2018
Merged

Tooltip issues from Lp1779468 #1752

merged 8 commits into from
Aug 9, 2018

Conversation

daschuer
Copy link
Member

Some tooltip fixes reported in https://bugs.launchpad.net/mixxx/+bug/1779468
It also equalizes the right click behavior on Samplers and preview decks for all skins.

I have changed the tooltips. The sampler right click follows CUE mode. Does it work?

@ronso0
Copy link
Member

ronso0 commented Jul 16, 2018

I'll test this.
Meanwhile I'm wondering why there are two identical Play buttons in Deere, one for wether [SamplerX],repeat is 0 or not
https://github.com/mixxxdj/mixxx/pull/1752/files#diff-60b5be35479b7006fc2a599dcf8d9522

@daschuer
Copy link
Member Author

In case of repeat enabled, the play button becomes violet. I have added a comment.

@daschuer
Copy link
Member Author

@ronso0: can we merge this now?

@daschuer daschuer added this to the 2.1.2 milestone Jul 28, 2018
@ronso0
Copy link
Member

ronso0 commented Aug 1, 2018

short on time..I'll have a look this evening

@ronso0
Copy link
Member

ronso0 commented Aug 2, 2018

the samplers work fine now.

LateNights missing CUE mark in sampler overview is added in #1754 but in Shadethey're missing:
cbe7580 Shade: add CUE mark to sampler overview
and IMO black marks are better there (if you like):
8663752 Shade: black Sampler overview marks

I noticed the decks' CUE right-click tooltip doesn't match:
all CUE buttons have the tooltip ID cue_default_cue_gotoandstop attached, right-click connection is cue_gotoandstop but AFAICT it shows the text of cue_default

@daschuer
Copy link
Member Author

daschuer commented Aug 3, 2018

Thank you for the commits. I have just merged them.

I do not see an issue with the right click Deck CUE buttons. Is it in Shade?

@ronso0
Copy link
Member

ronso0 commented Aug 3, 2018

hmm..its the same in all skins I suppose it will be solved when this is merged to 2.1.

got it: for the samplers's tooltip to work out (cue_gotoandplay_cue_default) you changed cueWhileStopped to "Set cue point (Pioneer/Mixxx/Numark mode), set cue point and play after release (CUP mode) OR preview from it (Denon mode)."
this is also called from cue_default_cue_gotoandstop which is used for decks' CUE buttons, but their right-click connection is cue_gotoandstop that's independent from play state of decks.
in 2.1 this points to correct tooltip "Seeks the track to the cue point and stops." and I suggest to keep https://github.com/mixxxdj/mixxx/pull/1752/files#diff-e131bb07ee88c7ad6d80f4643f558015L427

@daschuer
Copy link
Member Author

daschuer commented Aug 3, 2018

Sorry, I cannot follow. I have tested the 2.1 merge commit.
What should we do here next?

<< QString("%1: %2").arg(rightClick, tr("Seeks the track to the cue point and stops."));

add("pfl")
<< QString("%1: %2").arg(rightClick, cueWhileStopped);
Copy link
Member

Choose a reason for hiding this comment

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

sorry for the confusion. It's easy:
start this branch in dev mode, hover over a deck's CUE button in any skin.
check the right-click connection, have a look at the corresponding tooltip string below.
It doesn't match..

so here we should use QString cueWhilePlaying because all the deck CUE buttons have cue_gotoandstop as right-click connection

@daschuer
Copy link
Member Author

daschuer commented Aug 8, 2018

I have got it. It looks like I had accidentally removed the original right click tool tip.now it is back.
Merge?

@ronso0
Copy link
Member

ronso0 commented Aug 9, 2018

LGTM!

@daschuer
Copy link
Member Author

daschuer commented Aug 9, 2018

Thank you for help and final review!

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