Relates to D20533
Details
Details
Before:
After:
Diff Detail
Diff Detail
- Repository
- R102 KInfoCenter
- Branch
- arcpatch-D20598
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 10930 Build 10948: arc lint + arc unit
Comment Actions
way to align Kirigami.FormLayout together in a repeater.
Check out the twinFormLayouts property where you can have it manage multiple different FormLayout instances as a single layout
| Modules/energy/package/contents/ui/main.qml | ||
|---|---|---|
| 21 ↗ | (On Diff #56359) | and also getting rid of QQC1 |
| 29 | A goal is to get rid of these plasma imports, too | |
| 459 ↗ | (On Diff #56359) | Shouldn't be neccessary as it's QQC2.Label |
| Modules/energy/package/contents/ui/main.qml | ||
|---|---|---|
| 459 ↗ | (On Diff #56359) | For breeze dark this makes the label, dark gray on a dark background. See before screenshot. Could it be a breeze dark bug ? |
Comment Actions
Looks like you'll need to use twinFormLayouts. One unfortunate thing is you can't go "ha, I'll have all the layouts adjust to this specific one (with the longest label)". You'll need to have them all interlinked with one another because label length can change based on translation.
I can have a better look at it later in the day I hope.
Comment Actions
Here's what works:
Repeater {
id: helloThere // I'll leave the naming up to you :)
model: root.details
Kirigami.FormLayout {
twinFormLayouts: helloThere.itemAt(0)
EDIT: seems like it's not lining up the "Manufacturer" form heh.
| Modules/energy/package/contents/ui/main.qml | ||
|---|---|---|
| 30 ↗ | (On Diff #56359) | Now that you've imported Kirigami, you can use Kirigami.Units and drop the PlasmaCore import |
| 49 ↗ | (On Diff #56359) | there is trailing space in this row |
| 437 ↗ | (On Diff #56359) | trailing space her as well, as well as in the preceding row |
Comment Actions
This doesn't actually work for me. Upon opening, the following is printed to the console:
org.kde.kcoreaddons: Error loading plugin "kcm_energyinfo" "The shared library was not found."
Plugin search paths are ("/usr/lib/x86_64-linux-gnu/qt5/plugins", "/usr/bin")
The environment variable QT_PLUGIN_PATH might be not correctly set
<Unknown File>:3:89: Unable to assign [undefined] to double
QQmlExpression: Expression file:///usr/share/kpackage/kcms/kcm_energyinfo/contents/ui/main.qml:436:33 depends on non-NOTIFYable properties:
Solid::Battery::serialComment Actions
Works for me, nice work with aligning the layouts.
I'm a bit conflicted, but I think visually the issue is how these buttons look like with the port, as well as their icons (but that was a problem before as well):
Before they had a useless frame, but the buttons had a border on all 4 sides:
Losing the bottom maybe makes sense, but feels disconnected here IMO.
Comment Actions
Works for me, nice work with aligning the layouts.
I'm a bit conflicted, but I think visually the issue is how these buttons look like with the port, as well as their icons (but that was a problem before as well):
Before they had a useless frame, but the buttons had a border on all 4 sides:
Losing the bottom maybe makes sense, but feels disconnected here IMO.
I am using the dark theme and those buttons are pretty much
Works for me, nice work with aligning the layouts.
I'm a bit conflicted, but I think visually the issue is how these buttons look like with the port, as well as their icons (but that was a problem before as well):
Before they had a useless frame, but the buttons had a border on all 4 sides:
Losing the bottom maybe makes sense, but feels disconnected here IMO.
I don't see those buttons, I guess you need a remote mouse or something similar to have them.
| Modules/energy/package/contents/ui/main.qml | ||
|---|---|---|
| 23 ↗ | (On Diff #56359) | I might be too aggressive here, what would be the best version to set here ? |
| Modules/energy/package/contents/ui/main.qml | ||
|---|---|---|
| 23 ↗ | (On Diff #56359) | 2.5 is always a safe bet. |
Comment Actions
Yeah you would need 2 devices I guess. I have my laptop's battery and the wireless mouse's battery listed.
Comment Actions
I am now getting the same error as well, kcmshell5 is eating up one cpu core in the background.
Comment Actions
I am not focused on this anymore and might abandon this review as I really don't enjoy Qml hacking and lack of good tooling at least in my experience.
Comment Actions
If you still have some reserves of energy left, I'd really be appreciated! Someone seems to have fixed the icons so this patch now looks pretty good in 5.15:
It's a massive improvement over the current state. The only thing to would be to figure out what's causing the error with 5.16 and this would be good to go.
Comment Actions
What I've got right now is:
The most notable UI change is probably the fact that I added an inline message to waste less space.
So we still need to get rid of the top gap, have the window actually have correct minimum width, and look into that crash bug.
Comment Actions
Hey that looks pretty good!
To fix the top gap, I wonder if it would suffice to add anchors.fill: parent or some other positioning mechanism to the top-most ColumnLayout inside the Kirigami.ScrollablePage.
Comment Actions
That's what I thought, but it doesn't work :/
What we know is the gap was there before. Since we've now got a Kirigami.ScrollablePage, we can scroll all the content up by holding the mouse. And we can also just make the ScrollablePage the top item. Regardless of that, it shows a gap at the top. I don't see where in the QML code this is coming from.
BTW I think the errors were due to using a non-existent QQC2 import (2.12), so we're good on that front.
Comment Actions
Use KCM.SimpleKCM as root item, fix padding issues, increment kcm version, clean up old now unnecssary workaround
