| davidedmundson |
| Plasma |
if a framesvgitem has an array as prefix, like
FrameSvg {
prefix: ["toolbutton-hover", "hover"]
}
it will use the first available prefix, so on themes that
have toolbutton-hover, that one will be used, old themes will
continue to use "hover" as prefix
tested the calendar with breeze theme has breeze-widget style
toolbuttons, with olt themes as air, the old behavior is still
there, switching on the fly works
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| src/declarativeimports/core/framesvgitem.cpp | ||
|---|---|---|
| 286 | Maybe print a warning if none found? Or how did the old code behave where we would always call this no matter if we had it? | |
| 317 | Sure that QML gives you a QStringList? | |
| 429 | I think this loop which is in three places should be turned into a private method void applyPrefixes(const QStringList &prefixes) | |
| src/declarativeimports/core/framesvgitem.h | ||
| 122 | *can | |
[15:20] <notmart> if the svg doesn't exists, it falls back to the breeze one
[15:20] <notmart> then all hasElementPrefix would check if said prefix exists in the breeze theme
That's exactly what this code is doing, just in a much more complicated way...
no, because buttons.svg would exist in the old theme that does not have the prefix
| src/declarativeimports/core/framesvgitem.cpp | ||
|---|---|---|
| 286 | yeah, it would always do it (and for framesvg setting an inexistent prefix has the effect of setting an empty one) | |
| 317 | yep, just works | |
| 429 | ok | |
Either:
a QML client could just do
prefix: frame.hasElementPrefix("toolbutton-pressed") ? "toolbutton-pressed" : "pressed"or this doesn't work
It's the same thing
both are just using for m_frameSvg->hasElementPrefix()
ah, yes, of course.
(since the irc discussion you quoted was about falling back between files, i tought that was the topic, while here is about falling back between prefixes)
anyways yes, that qml chunk does the same thing, tough it would not work when you change the theme from one that has the prefix to one that hasn't, as it's querying a method so can't be binded. (and is uglier qml, that being the part that breaks more easily, i think is better to have uglier c++ in order to have prettier qml)
note there is a minor API break for any JS code that does
if (prefix === "someValue")
but I've checked and no-one does...(and it would be a bit of a weird thing to do)
| src/declarativeimports/core/framesvgitem.cpp | ||
|---|---|---|
| 571–574 | this !found is a behavioural change on the existing code. Why are we adding it? | |
| src/declarativeimports/core/framesvgitem.cpp | ||
|---|---|---|
| 571–574 | before it did set the elementprefix on the framesvg regardless it was present or not, so this is done in order precisely to not do a behavioral change. to be more exact to be sure no behavioral change occurred at all, i would have to do if (!found) { m_frameSvg->setElementPrefix(m_prefixes.last()); } which if doesn't exist, is pretty much the same as setting an empty string, but i can change it to do so | |