Kicker: Menu Categories have weird spacing (compared to Plasma1)

[star-buck]

same font, even if latter is diamond theme, it would look the same on plasma1 as the one in the image on top.

[eikehein]

It's fine with Breeze though, so it has to be the theme. I'll install Diamond and see what's happening there. My hunch is that Diamond's highlight SVG item has incorrect margin hints.

[eikehein]

Can't reproduce with Diamond:

Can anyone else?

[notmart]

Here is with Diamond: spacing seems fine. maybe is the reported DPI?

[star-buck]

idea: having tested this theme before Diamond, the spacing might come from this theme and somehow not recover when changing to Diamond: http://netrunnerlinux.com/plasma5/Enlightenment.rar While Enlightenment works fine in Plasma1 Kicker, can someone confirm it looks weird in Plasma5 Kicker and identify whats causing it?

[notmart]

Bingo, It seems the defective theme is indeed Enlightenment, and shows here the kicker menu as cramped as the screenshot. changing theme the spacing doesn't change, so both for instance breeze and diamond show then the same problem. restarting fixes it. also, creating a new instance of kicker, the new one will have correct spacing. this suggests there is some signals that are not either emitted or received down the stack, that doesn't make the margins of the new graphics being re-read dinamically

[eikehein]

Kicker code:

PlasmaCore.FrameSvgItem {
    id : highlightItemSvg

    visible: false

    imagePath: "widgets/viewitem"
    prefix: "hover"
}

property int itemHeight: (Math.max(theme.mSize(theme.defaultFont).height, units.iconSizes.small)
    + highlightItemSvg.margins.top + highlightItemSvg.margins.bottom)

-> I'm guessing FrameSvgItem doesn't emit a change signal for margins.

[notmart]

seems it does, but i see here it's emitting 0 as new value, will investigate why

[notmart]

what seems happening is that enlightenment doesn't have the "hover" prefix, so going from one that doesn't to one that has is as it doesn't notice the new theme has the element the old hadn't (if plasma is started with breeze, then switch to enlightenment, then switch back to breeze, all works correctly)

[notmart]

It should be fixed now in plasma-framework master

[eikehein]

Good fix, thanks!

Tracking note: Frameworks fix means it won't be in 5.0 (but next week's ISO will have the fix from git).

I think we can close here.

[star-buck]

Enlightenment doesnt have a hover state for tasks on purpose... Kicker1 still worked fine, has something in Kicker2 (aka new Kicker) changed regarding how themes are applied, since it also doesnt seem to hover menu items?

Greetings, Clemens. On Jul 11, 2014 4:54 PM, "eikehein" notifications@github.com wrote:

Good fix, thanks!

Tracking note: Frameworks fix means it won't be in 5.0 (but next week's ISO will have the fix from git).

I think we can close here.

— Reply to this email directly or view it on GitHub https://github.com/blue-systems/plasma-5.0/issues/9#issuecomment-48740307 .

[eikehein]

Yeah, there are semantic changes. Kicker 1 used to take the margins from widgets/listitem:normal (themelement:prefix), but Kicker 2 takes them from widgets/viewitem:hover (which is what PlasmaComponents.Highlight uses).

The thing is, you need to decide on the one or the other so that the list items don't change size when they're hovered, and IIRC in Breeze the margins for widgets/viewitem:hover are bigger than for widgets/listitem:normal, so I went with the former to avoid size jumps.

What I can do though is adjust the code and make it use the max() of the one or the other, so that if the margins for widgets/viewitem:hover (as in Enlightenment) are zero, it falls back to widgets/listitem:normal's margins, to behave well in both cases.

[eikehein]

Change pushed, Kicker will now fall back to using listitem:normal margins when viewitem:hover's are zero.

[star-buck]

What does the menu hover element have to do with the taskmanager hover? Both can still be done seperate anyway?

[eikehein]

Screenshot with Enlightenment now:

[eikehein]

What does the menu hover element have to do with the taskmanager hover?

Nothing, the Task Manager (or the task hover element) are not involved here.

The Enlightenment theme has no widgets/viewitem:hover (tasks would be widgets/task:hover).

widget/viewitem is what Plasma's PlasmaComponents.Highlight uses, which is used for hovers in list views.

So yeah, it can be themed separately and the theme currently doesn't have list hover elements. If it wants one it needs to add a widgets/viewitem:hover. It's a theme bug.

The bugs on our side were:

• Plasma's theming system not handling going from theme with the element to without it and back well (because what theme doesn't have this ... easy to forget).
• Kicker not trying harder to get a useful margin when the current theme is incomplete.

[eikehein]

Clemens messaged me today that this problem has returned, and indeed I see it as well, even though the Kicker code is unchanged.

Let's recap:

1. Kicker 1 used to use margins from widgets/listitem:normal in calculating the delegate size.
2. Kicker 2 switched to widgets/viewitem:hover, because that's what PlasmaComponents.Highlight uses, and in Breeze widgets/viewitem:hover has bigger margins than widgets/listitem:normal, so to avoid size jumps on hover ...
3. The Enlightenment theme is technically incomplete and provides no widgets/viewitem:hover. I consider this a bug regarding completeness of basic elements in the theme (if it doesn't want to show anything it should provide an invisible item IMHO, not delete the prefix). Nonetheless, I added fallback code in Kicker to go back to widgets/listitem:normal if this is the case, or rather use the max() of the two elements' margins.
4. Now it gets interesting: The Enlightenment theme actually has no widgets/listitem(.svgz) either. It seems that both Plasma 4.x and Plasma 5.0 get a listitem from somewhere else in this case (fallback to default, I guess). This no longer happens on 5.1 / master, so Kicker can't get margins at all and rows are tiny.

Now, I still consider this another bug in the theme, and by far the easiest fix is to just make the theme complete. OTOH, a behavior change in the theming system is interesting regardless. Marco?

(Note: I haven't done any actual debugging on this yet because I wasn't at the desk today.)

[eikehein]

An interesting behavior change in the theming system: Enlightenment's viewitem.svgz only has one element group with no prefixes (i.e. just left, center, etc.) and apparently FrameSvgItem used to fall back to this even when a specific prefix was requested. That's why PlasmaComponents.Highlight (which uses widgets/viewitem:hover) worked in Plasma 4 with the Enlightenment theme but no longer works in Plasma 5.

This doesn't explain where Kicker 1 got widgets/listitem margins from, so that's still a mystery. But if this behavior was still there, Kicker 2 could have gotten the margins from widgets/viewitem in the first place.

This could be a real bug in the theming system IMHO - other themes might also rely on providing prefix-less elements ...

[notmart]

Ok, this week i'll try to bisect to see when the behavior changed and why

[notmart]

rendering of the item from the enlightenment theme is fixed. may need a minor adjustment in kicker for the spacing

[eikehein]

Kicker adjusted as per IRC discussion, too; closing.