support for user removing background and automatic shadow
ClosedPublic

Authored by mart on Nov 28 2019, 1:49 PM.

Details

Summary

add the ability for the user to override the background hints anddecide
about applets having background or a shadow
A new handle button will be added to manually enable/disable background for
plasmoids that support it
if they don't, the ImmutableBackground flag is set in the hints.

PlasmaCore.ColorScope has been expanded to work more like Kirigami.Theme (which will be replaced by) even if it's oòld behavior still works. This makes the complementary colorscope in applets with shadow actually work (and at the same time gives a clearer porting path for kf6)

Test Plan

Tested the functionality and correct save/restore

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Nov 28 2019, 1:49 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 28 2019, 1:49 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
mart requested review of this revision.Nov 28 2019, 1:49 PM
mart added inline comments.Nov 28 2019, 2:01 PM
src/plasma/plasma.h
285

DefaultBackground should be StandardBackground|ImmutableBackground
in order to have this feature only for applets that explicitly support this (some applets just look silly or kinda broken due to the limited use of ColorScope around, so this shouldn't be supported unless the applet explicitly says so)

strictly speaking should be bc, as existing binaries shouldn't fail with it (tough just believe the default is a different one which can give problems as before the user code was checking for hints === something rather than hints&something

So probably this flag based approach isn't much usable and different api may be needed?

mart edited the summary of this revision. (Show Details)Nov 28 2019, 2:03 PM
davidedmundson requested changes to this revision.Nov 28 2019, 3:35 PM
davidedmundson added subscribers: ndavis, davidedmundson.
davidedmundson added inline comments.
src/declarativeimports/core/colorscope.cpp
52–57

It' be better to split this colour change stuff.

It's effectively an unrelated bugfix/cleanup

110

typo

src/desktoptheme/breeze/widgets/configuration-icons.svg
17–18

@ndavis can you review this icon change please

src/plasma/plasma.h
284

the user shouldn't have the possibility to.....

src/scriptengines/qml/plasmoid/appletinterface.cpp
201–202

0 == NoBackground which is a perfectly valid entry

I think we need to use the
keyToValue(QString, bool*) overload and check the return value.

This revision now requires changes to proceed.Nov 28 2019, 3:35 PM
mart updated this revision to Diff 70503.Nov 28 2019, 3:50 PM
mart marked 3 inline comments as done.
  • adress comments
mart updated this revision to Diff 70513.Nov 28 2019, 5:11 PM
  • ImmutableBackground->configurableBackground

That ended up quite nice in the end ++

src/scriptengines/qml/plasmoid/appletinterface.cpp
416

I think technically has a bug.

if m_userBackgroundHintsInitialized == true

and the m_backgroundHints changed from !configurable -> configurable

then the effectiveBackgroundHints will change, but we're not emitting it.

i.e this should be

if (!m_userBackgroundHintsInitialized || 
        !(newbackgroundHints & Plasma::Types::ConfigurableBackground))
        !(oldbackgroundHints & Plasma::Types::ConfigurableBackground)

Though personally I would just call effectiveBackgroundHints() before and after, it's more robust than duplicating the logic.

ndavis added inline comments.Nov 29 2019, 3:19 AM
src/desktoptheme/breeze/widgets/configuration-icons.svg
17–18

I couldn't see any issues in the SVG code. @mart Where is showbackground used in the UI so that I can test it?

mart added inline comments.Nov 29 2019, 8:50 AM
src/desktoptheme/breeze/widgets/configuration-icons.svg
17–18

in the patch, it adds a new icon in the handle, tough you need this plus D25591 and D25592 to have it working.
the idea is to have this button that can add and remove the background from a plasmoid switching between the full frame and inverted colors with drop shadow.
right now is like an a with a rectangular background behing, that could mean literally anything (was also thinking to switch the icon between this one and an a with a shadow without rectangle) it's shown in the screenshot in D25591

tough i fear there doesn't exist an icon that can communicate this unambiguously

mart updated this revision to Diff 70544.Nov 29 2019, 9:22 AM
  • better effectiveBackgroundHints comparizon
mart marked an inline comment as done.Nov 29 2019, 9:22 AM
mart added inline comments.
src/desktoptheme/breeze/widgets/configuration-icons.svg
17–18

that's the context it's used in:

davidedmundson accepted this revision.Nov 29 2019, 9:23 AM
This revision is now accepted and ready to land.Nov 29 2019, 9:23 AM
This revision was automatically updated to reflect the committed changes.