MuseScore icon indicating copy to clipboard operation
MuseScore copied to clipboard

Added error after adding interval with no note selected

Open randoguyname opened this issue 2 years ago • 9 comments

Shows an error if the "add interval" function is called when no note or chord is selected. Resolves: #13296

  • [x] I signed the CLA
  • [x] The title of the PR describes the problem it addresses
  • [x] Each commit's message describes its purpose and effects, and references the issue it resolves
  • [x] If changes are extensive, there is a sequence of easily reviewable commits
  • [x] The code in the PR follows the coding rules
  • [x] There are no unnecessary changes
  • [x] The code compiles and runs on my machine, preferably after each commit individually
  • [ ] I created a unit test or vtest to verify the changes I made (if applicable)

randoguyname avatar Dec 19 '22 09:12 randoguyname

Tested #13296 on Win10

@randoguyname I see the dialog's shape and some text style a little bit different comparing with other dialog. Can you change it to make more similar?

https://user-images.githubusercontent.com/90187801/212703358-c5c80650-e7a1-418a-ba4b-840afe310470.mp4

DmitryArefiev avatar Jan 16 '23 14:01 DmitryArefiev

@DmitryArefiev The shape is determined by the width of the text. We could give these message dialogs a minimum width, but I'm not sure if that's in the scope of this PR.

And about the text style, I believe that this new dialog follows the desired design more closely than the one about staff texts.

cbjeukendrup avatar Jan 16 '23 15:01 cbjeukendrup

@DmitryArefiev The shape is determined by the width of the text. We could give these message dialogs a minimum width, but I'm not sure if that's in the scope of this PR.

And about the text style, I believe that this new dialog follows the desired design more closely than the one about staff texts.

It just seems to tight for me)

Capture2

@bkunda @jessjwilliamson Please review

DmitryArefiev avatar Jan 16 '23 19:01 DmitryArefiev

I'd be more worried about having "Don't show this again" on a dialog that's the only feedback you get that you've tried to use a command when it doesn't make sense...

wizofaus avatar Jan 16 '23 19:01 wizofaus

I'd like to point out that this isn't a new dialog at all, rather an application of the existing no note selected dialog.

randoguyname avatar Jan 17 '23 02:01 randoguyname

I'd be more worried about having "Don't show this again" on a dialog that's the only feedback you get that you've tried to use a command when it doesn't make sense...

@wizofaus I think that's fine since the checkbox is not checked by default

DmitryArefiev avatar Jan 17 '23 14:01 DmitryArefiev

I'd like to point out that this isn't a new dialog at all, rather an application of the existing no note selected dialog.

@randoguyname Got it. Thanks! I'll create a separate issue for inconsistency in dialog style

DmitryArefiev avatar Jan 17 '23 14:01 DmitryArefiev

Tested #13296 on Win10, Mac12, LinuxMint 22.04 - FIXED

DmitryArefiev avatar Jan 17 '23 14:01 DmitryArefiev

@cbjeukendrup Should this be rebased and reviewed again? Or we can merge into master now?

DmitryArefiev avatar Jan 17 '23 14:01 DmitryArefiev

The change is quite trivial enough and I don't think there have been any potentially conflicting other changes in the meantime, so I'd say it's fine!

cbjeukendrup avatar Jan 17 '23 18:01 cbjeukendrup

@cbjeukendrup Cool. Thanks!

DmitryArefiev avatar Jan 18 '23 10:01 DmitryArefiev