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

Fix #305387: Add spinner for controlling glissando line thickness in inspector #7435

Merged

Conversation

sidharth-anand
Copy link
Contributor

Resolves: https://musescore.org/en/node/305387

This patch allows the user to directly change the thickness of the glissando line from the inspector

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • I created the test (mtest, vtest, script test) to verify the changes I made

@cbjeukendrup
Copy link
Contributor

cbjeukendrup commented Feb 5, 2021

I tested it and noticed two things:

  • If you select a glissando that is already "wavy", the line thickness control is (erroneously) shown.
    However, if you select a glissando that is "straight" and then change it to "wavy", the line thickness control is correctly hidden (and comes back again if you change it back to "straight").
  • The line thickness control shows no suffix / measurement unit. It would be nice if it would say 0.15sp instead of just 0.15. I believe this is supported out-of-the-box in Qt Creator, so you could just look at the properties of other spinboxes that do show sp.

@sidharth-anand
Copy link
Contributor Author

I'll update the pr with these fixes

@cbjeukendrup
Copy link
Contributor

Ah, for the first one, it looks like you need to add this:

void InspectorGlissando::setElement()
      {
      InspectorElementBase::setElement();
      if (!g.playGlissando->isChecked()) {
            g.labelGlissandoStyle->setEnabled(false);
            g.glissandoStyle->setEnabled(false);
            g.resetGlissandoStyle->setEnabled(false);
            }
      if (!g.showText->isChecked())
            g.textWidget->setVisible(false);
+     g.lineWidthWidget->setHidden(g.type->currentIndex() == 1);
      g.easeInSlider->setSliderPosition(g.easeInSpin->value());
      g.easeOutSlider->setSliderPosition(g.easeOutSpin->value());
      g.easeInOutCanvas->setEaseInOut(g.easeInSpin->value(), g.easeOutSpin->value());

      updateEvents();
      }

@sidharth-anand sidharth-anand force-pushed the 305387-glissando-thickness-control branch from 9ff4be8 to 7877f30 Compare February 5, 2021 16:19
@sidharth-anand
Copy link
Contributor Author

Updated with both the fixes

@Jojo-Schmitz
Copy link
Contributor

Needs #7704

@Jojo-Schmitz
Copy link
Contributor

I wonder whether this would need to get ported to master

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.

4 participants