Skip to content

Conversation

@octopols
Copy link
Contributor

@octopols octopols commented Feb 3, 2023

Resolves: #13652
Color picker now have alpha value. If the alpha value is 0 and the color picker is launched the value is changed to 255 automatically.

Sorry I had to close the last PR fixing this issue because of me deleting my original fork.
This PR is a replacement for #16103 .

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • 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)

@octopols
Copy link
Contributor Author

octopols commented Feb 3, 2023

issue_13652_fix.mp4

@cbjeukendrup cbjeukendrup self-requested a review February 3, 2023 12:18
@octopols
Copy link
Contributor Author

octopols commented Mar 2, 2023

Is there a way for me to add a checkbox to QColorDialog to enable or disable checkbox? When I first tackled this issue I couldn't find the qml for QColorDialog and soon realized it's an inbuit component and never really looked into editing the dialog's UI.

@cbjeukendrup
Copy link
Member

cbjeukendrup commented Mar 4, 2023

@octopols What do you mean? The color dialog is platform-dependent, i.e. Qt just calls the OS's color dialog. That means that you don't have any possibilities to customise it, beyond the options offered by Qt.

Copy link
Member

@cbjeukendrup cbjeukendrup left a comment

Choose a reason for hiding this comment

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

I see one issue with this PR: probably not every color picker should have an alpha channel. For example, I think the color pickers in Preferences > Appearance should not have an alpha channel, because that may cause strange results.

So I think you need to add a property to the ColorPicker component in QML to enable/disable the alpha channel per instance of ColorPicker. Then, you need to propagate the value of that property via ColorPickerModel to Interactive::selectColor, which should get a new parameter for that. Then, based on that parameter, you should pass QColorDialog::ShowAlphaChannel or QColorDialog::ColorDialogOptions() to the QColorDialog.

@octopols
Copy link
Contributor Author

octopols commented Mar 4, 2023

I was discussing this issue with @VanSHOE recently, and he suggested editing it through Qt Quick components. However, I'm not entirely sure if I conveyed my problem correctly to him.
I will implement the fix you mentioned. However, if we do decide to add a checkbox to enable the alpha channel, is our only option to design a completely new component?

@cbjeukendrup
Copy link
Member

cbjeukendrup commented Mar 4, 2023

I'm afraid yes...

I would prefer to use the platform dialogs as much as we can, because they "just work" and people already know them from other apps.

@octopols
Copy link
Contributor Author

octopols commented Mar 4, 2023

@cbjeukendrup I'm having a really hard time propagating a bool I created showAlpha to Interactive::selectColor. I think I need to make some changes in src\framework\uicomponents\view\colorpickermodel.cpp but I don't even know where to start. Should I commit the changes I made so far?

@octopols
Copy link
Contributor Author

octopols commented Mar 4, 2023

Screenshot 2023-03-05 003235

@cbjeukendrup
Copy link
Member

cbjeukendrup commented Mar 4, 2023

That list of files looks good. The "missing link" will indeed be in colorpickermodel.h/cpp. I think the easiest way is to add a showAlpha bool parameter the method Q_INVOKABLE QColor selectColor(const QColor& currentColor);.

@octopols
Copy link
Contributor Author

octopols commented Mar 4, 2023

I made some changes in the colorpickermodel.h/cpp files but I'm still getting an error in the ColorPicker.qml file.
ColorPicker qml

These are the changes I made in colorpickermodel.h
colorpickermodel h

And these are the changes I made in colorpickermodel.cpp
colorpickermodel cpp

@cbjeukendrup
Copy link
Member

Ah, the problem is that showAlpha is not a property of the ColorPickerModel class, but a parameter of the selectColor method.

That method is called here:

    QtObject {
        id: prv

        function selectColor() {
            var selectedColor = colorPickerModel.selectColor(root.color) // <-- here
            root.newColorSelected(selectedColor)
        }
    }

So in that place, you need to pass the showAlpha parameter.

@octopols
Copy link
Contributor Author

octopols commented Mar 4, 2023

I have this one last problem (hopefully), how do I set the value of showAlpha from TextSettings.qml.
how_to_set

@cbjeukendrup
Copy link
Member

There are two solutions:

  1. setting the property of the color picker inside ColorSection.qml (but that means that it will be enabled for all color pickers that are created using ColorSection; not sure if that's desired
  2. create a showAlpha property alias in ColorSection.qml: inside the root item of the file, you would add property alias showAlpha: colorPicker.showAlpha.
    After giving ColorSection this property, you can go to TextSettings.qml and use it like just showAlpha: true.

@octopols
Copy link
Contributor Author

octopols commented Mar 5, 2023

Thank you very much for your help. The issue has been resolved now.

@octopols octopols force-pushed the octopols/issue_13652_fix branch from fe67b8f to 5b88118 Compare March 5, 2023 02:15
@cbjeukendrup cbjeukendrup self-requested a review March 5, 2023 12:20
@MarcSabatella
Copy link
Contributor

Sorry if this was already obvious to everyone else, but in case it's useful or relevant - I just realized, the alpha channel control already is present if you involve the color picker from the Format / Style / Text Styles. Which is great, because that's where it is actually needed most. But of course, it would be great to have in Properties as well

@Jojo-Schmitz
Copy link
Contributor

Rebase needed

@octopols
Copy link
Contributor Author

Rebase needed

I'll rebase it today, sorry for replying late

@igorkorsukov igorkorsukov force-pushed the master branch 6 times, most recently from fa1f8d3 to 525a11a Compare February 14, 2024 09:08
@cbjeukendrup cbjeukendrup force-pushed the octopols/issue_13652_fix branch from 5b88118 to 1fac39a Compare November 4, 2025 15:37
@cbjeukendrup cbjeukendrup changed the title Fix for Issue #13652 MU4 Issue] Regression: opacity control missing in color picker Fix #13652: Show opacity control in color picker where appropriate Nov 4, 2025
@cbjeukendrup cbjeukendrup force-pushed the octopols/issue_13652_fix branch from 1fac39a to 8942698 Compare November 4, 2025 15:39
@cbjeukendrup cbjeukendrup requested review from mathesoncalum and removed request for cbjeukendrup November 4, 2025 15:40
@cbjeukendrup
Copy link
Member

@zacjansheski please check that the colour pickers in the properties panel now have a (working) alpha channel, i.e. the one inside the Appearance popup, and the ones for text border / fill.

@zacjansheski
Copy link
Contributor

I'm unable to open these builds, tried on both MacOS and Windows. Splash screen and then MSS crashes.

@cbjeukendrup please advise

- `IInteractive::selectColor` gets an `allowAlpha` parameter
- `ColorPicker` gets a property whose value is passed to this new parameter
- When alpha is 0 and the color dialog is launched, the alpha is changed to 255 in the color dialog, to prevent confusion ("Hm, I chose a color but I still see nothing...")

Co-Authored-By: Casper Jeukendrup <[email protected]>
@cbjeukendrup cbjeukendrup force-pushed the octopols/issue_13652_fix branch from 8942698 to 3518ff3 Compare November 6, 2025 21:04
@cbjeukendrup
Copy link
Member

It turns out I had not properly removed a change from an old version of this PR... should be fixed now!

@zacjansheski
Copy link
Contributor

Tested on MacOS 15, Windows 11. Approved
#13652 FIXED

@cbjeukendrup cbjeukendrup merged commit 8a9ab10 into musescore:master Nov 6, 2025
12 checks passed
@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Nov 7, 2025
@Jojo-Schmitz
Copy link
Contributor

Backporting to 4.6.4?

@cbjeukendrup
Copy link
Member

Hm... yeah, I'm not against.

This was referenced Nov 7, 2025
@Jojo-Schmitz
Copy link
Contributor

Done, see #30918

@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Nov 10, 2025
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.

Regression: opacity control missing in color picker

6 participants