Components that used QQC1 in sddm-theme/ (excluding the symlinked components folder) have been changed to use QQC2.
Details
- Reviewers
davidedmundson filipf - Group Reviewers
Plasma VDG - Maniphest Tasks
- T10958: Faster Startup
- Commits
- R120:5c8ce892e394: [sddm-theme] Start moving from QQC1 to QQC2
Ensure no loss of functionality, style, or bugs from the port of QQC1 to QQC2.
Diff Detail
- Repository
- R120 Plasma Workspace
- Branch
- arcpatch-D21815
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 15438 Build 15456: arc lint + arc unit
Looks like this change managed to hide from Arcanist's gaze. Should have been there in the first place.
Concept makes sense, minor tweaks above
Make sure you test in full sddm, the test mode will infer some settings from your session which won't be valid in real usage.
sddm-theme/KeyboardButton.qml | ||
---|---|---|
22–26 | Please fix. | |
sddm-theme/SessionButton.qml | ||
28 | Why the wrapper? | |
30 | Implicit height should be propagated upwards, not height. |
Updates based off of feedback, and adding the standard menu fade animation to the QQC2 menus.
sddm-theme/SessionButton.qml | ||
---|---|---|
28 | QML files can only have one root, and the ToolButton's menu property doesn't accept QQC2 menus, meaning they have to be separated. |
Seems like you have all the QQC1 imports covered. I'm seeing a couple more in other files that are unused so it would be perfectly fine to remove them as a part of this diff.
I'll give the new code a spin soon and then I can add more detailed comments.
sddm-theme/SessionButton.qml | ||
---|---|---|
23 | Note that (confusingly) Plasma components 2 is QQC1 It might fix the menu issue | |
30 | I meant implicitWidth: toolBtn.implicitWidth width by default is bound to the implicitWidth so you don't need to set it |
sddm-theme/SessionButton.qml | ||
---|---|---|
23 | The menu property seems to be removed in PC3 |
sddm-theme/SessionButton.qml | ||
---|---|---|
23 | Additionally, ToolButton still seems to use QQC1 even in PC3. |
Added some comments that apply to both button files.
On a final note, watch out for whitespace because there's lots of it that's been added. You can turn on whitespace displaying in Kate:
sddm-theme/SessionButton.qml | ||
---|---|---|
28 | It seems to be running fine without the Item wrapper for me. What I did though was but the QQC2 menu inside the button, I didn't split them up. | |
44 | While we're doing this, can you perhaps make the menu pop up *above* the button? | |
57 | This can be up for debate, but I sort of liked the older behavior more when the menu's width would be determined by the size of the ToolButton at a particular point. | |
93 | Why do we need to set this? |
sddm-theme/SessionButton.qml | ||
---|---|---|
44 | It seems that changing the y value doesn't affect its y position for whatever reason. |
- Remove Item {} root and embed QQC2.Menu {} in PlasmaComponents.ToolButton{}
- If toolbutton is larger than largest menu item, resize menu to size of toolbutton
- Other minor things
sddm-theme/KeyboardButton.qml | ||
---|---|---|
10 | Prefer descriptive variable names; toolButton is better than toolBtn, but even better would be a description of what this button is actually for. The original was better and didn't need to be changed IMO. |
For positioning the menu maybe you could have a look at how it was done in Kickoff config window: https://github.com/KDE/plasma-desktop/blob/master/applets/kickoff/package/contents/ui/ConfigGeneral.qml
The menu seems to be extremely stubborn when it comes to y positioning. No amount of properties set or values passed to popup will get it to budge from its y position, including the method Kickoff uses.
- Change toolBtn to be more descriptive based on file the id was in
- Styling changes on menus (they now have shadows)
- Menu item highlight is tweaked as per T11124 (can revert if shouldn't be here)
If keeping the new menu style we have to wait until all those patches get merged in Plasma.
Might be better to just follow current style; updating it to the new one would be a quick review.
LGTM in use. The only thing I'd ask you to do is add font.pointSize: config.fontSize to both of the QQC2.Labels you were working on. SDDM defaults to 9pt font sizes; we need to override that so it's 10pt (same as the Plasma default).
Is this screenshot outdated? Why does the selected item extend outside the width of the list?