Add "Get Wallpaper Plugins..." button to Config Desktop dialog
ClosedPublic

Authored by Zren on Apr 8 2018, 6:39 AM.

Details

Summary

Patch 2/2 (other patch is D12040 for plasma-workspace)
Depends on D12040

FEATURE: 386621

Not sure I should be depending on the "image" C++ plugin here.

Screenshot:

Which opens this GHNS window from Patch 1:

TODO:

  • Move C++ plugin to components/shellprivate/wallpaperplugin ? I probably need to move the new GHNS code to components/shellprivate/wallpaperplugin since "desktoppackage" has the widgetexplorer C++ plugins there. I figured I'd put up a working patch first.
  • Update wallpaperComboBox and configDialog.wallpaperConfigModel so we don't have to close and reopen the "Configure Desktop" window for the plugin to show up in the dropdown.
Test Plan
  1. Install both patches
  2. Configure Desktop > Get Wallpaper Plugins > Install something (eg: Ken Burns Effect)
  3. Confirm it showed up in ~/.local/share/plasma/wallpapers/
  4. Close GHNS dialog and Configure Desktop dialog
  5. Reopen Configure Desktop
  6. Confirm "Ken Burns Effect" shows up

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
Zren created this revision.Apr 8 2018, 6:39 AM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 8 2018, 6:39 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham edited the summary of this revision. (Show Details)Apr 8 2018, 2:57 PM
ngraham edited the summary of this revision. (Show Details)
ngraham added a subscriber: ngraham.Apr 8 2018, 3:04 PM
ngraham added inline comments.
desktoppackage/contents/configuration/ConfigurationContainmentAppearance.qml
70–71

Don't leave commented-out code; remove it

74

Don't leave commented-out code; remove it

154

It might be a bit wordy, but to be consistent with the form used everywhere else, this should say, "Get new Wallpaper Plugins..."

Zren added inline comments.Apr 8 2018, 3:43 PM
desktoppackage/contents/configuration/ConfigurationContainmentAppearance.qml
154

"Get New Wallpaper" is capitalized. Are we using lowercase so the "Wallpaper Plugins" stands out more?

lowercase new vs uppercase New

ngraham added inline comments.Apr 8 2018, 9:56 PM
desktoppackage/contents/configuration/ConfigurationContainmentAppearance.qml
154

Sorry for the confusion, I was more commenting on the fact that the word "New" would be included. Following the pattern is a good idea, though I would favor lowercasing the word "new" everywhere (would be in another patch though).

Zren updated this revision to Diff 31700.Apr 8 2018, 10:02 PM

Remove commented code.
Add " New " to button label.

Is this still a WIP or ready for review? Please update the title when it is ready.

Zren updated this revision to Diff 32246.Apr 16 2018, 4:22 AM
Zren retitled this revision from [WIP] Add "Get Wallpaper Plugins..." button to Config Desktop dialog to Add "Get Wallpaper Plugins..." button to Config Desktop dialog.
Zren edited the summary of this revision. (Show Details)

Change from Wallpaper.Image to the ShellPrivate.WallpaperPlugin

Zren added a comment.Apr 16 2018, 4:40 AM

I'm still unsure how to update the dropdown after you download a new plugin.

I've moved the "open GHNS window" function in plasma-workspace to components/shellprivate/wallpaperplugin, but I've just noticed the dropdown is populated by configDialog.wallpaperConfigModel

which is populated here:

Code that here is fine.
As you say we need a tidy way to reload that list in order to have this mergable. I'll have a think.

abetts added a subscriber: abetts.Apr 16 2018, 2:31 PM

Why not save the room on the right of the KCM and instead blur the actual image in the list? Move the controls closer to that list item? The arrangement looks off the way it shows in the screenshots.

Zren added a comment.Apr 16 2018, 2:50 PM

Why not save the room on the right of the KCM and instead blur the actual image in the list? Move the controls closer to that list item? The arrangement looks off the way it shows in the screenshots.

If you're referring to my inactiveblur wallpaper plugin, it's not part of the patch.

But anyways, blurring the selected image in the file picker is a neat idea. I do like having the non blurred image and the blurred image side by side though. The UI is slightly modified from the animatedhue config, which needed the preview to test the 360* colour wheel, so I didn't put much thought into it.

In D12041#247368, @Zren wrote:

Why not save the room on the right of the KCM and instead blur the actual image in the list? Move the controls closer to that list item? The arrangement looks off the way it shows in the screenshots.

If you're referring to my inactiveblur wallpaper plugin, it's not part of the patch.

But anyways, blurring the selected image in the file picker is a neat idea. I do like having the non blurred image and the blurred image side by side though. The UI is slightly modified from the animatedhue config, which needed the preview to test the 360* colour wheel, so I didn't put much thought into it.

Cool!

mart added a subscriber: mart.May 16 2018, 10:41 AM

what's the status of this?
when ready, should go in, together D12040 at the beginning of 3.14 vyvle

Sorry for the delay

Update: Spoke to kpackage maintainer.
I'll add an anonymous DBus signal in kpackagetool and a PackageWatcher to monitor that.

Then when I get that hooked up, the blocking problem will be solved.

davidedmundson accepted this revision.Aug 30 2018, 11:18 PM

though only merge when we're ready.

This revision is now accepted and ready to land.Aug 30 2018, 11:18 PM
This revision was automatically updated to reflect the committed changes.