D12041: [WIP] Add "Get Wallpaper Plugins..." button to Config Desktop dialog

2018-04-11 Thread David Edmundson 
davidedmundson added a comment.


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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12041

To: Zren, #plasma
Cc: davidedmundson, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12041: [WIP] Add "Get Wallpaper Plugins..." button to Config Desktop dialog

2018-04-08 Thread Chris Holland 
Zren updated this revision to Diff 31700.
Zren added a comment.


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

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12041?vs=31652&id=31700

REVISION DETAIL
  https://phabricator.kde.org/D12041

AFFECTED FILES
  desktoppackage/contents/configuration/ConfigurationContainmentAppearance.qml

To: Zren, #plasma
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12041: [WIP] Add "Get Wallpaper Plugins..." button to Config Desktop dialog

2018-04-08 Thread Nathaniel Graham 
ngraham added inline comments.

INLINE COMMENTS

> Zren wrote in ConfigurationContainmentAppearance.qml:159
> "Get New Wallpaper" is capitalized. Are we using lowercase so the "Wallpaper 
> Plugins" stands out more?
> 
> lowercase new vs uppercase New
> 
> F5800706: 2018-04-08___11-09-40.png  
> F5800707: 2018-04-08___11-10-01.png 

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).

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12041

To: Zren, #plasma
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12041: [WIP] Add "Get Wallpaper Plugins..." button to Config Desktop dialog

2018-04-08 Thread Chris Holland 
Zren added inline comments.

INLINE COMMENTS

> ngraham wrote in ConfigurationContainmentAppearance.qml:159
> It might be a bit wordy, but to be consistent with the form used everywhere 
> else, this should say, "Get new Wallpaper Plugins..."

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

lowercase new vs uppercase New

F5800706: 2018-04-08___11-09-40.png  
F5800707: 2018-04-08___11-10-01.png 

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12041

To: Zren, #plasma
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12041: [WIP] Add "Get Wallpaper Plugins..." button to Config Desktop dialog

2018-04-08 Thread Nathaniel Graham 
ngraham added inline comments.

INLINE COMMENTS

> ConfigurationContainmentAppearance.qml:71
>  spacing: units.largeSpacing / 2
> -anchors.right: wallpaperRow.right
> +// anchors.right: wallpaperRow.right
>  visible: pluginComboBox.count > 1

Don't leave commented-out code; remove it

> ConfigurationContainmentAppearance.qml:76
> +// height: parent.height
> +// }
>  QtControls.Label {

Don't leave commented-out code; remove it

> ConfigurationContainmentAppearance.qml:159
> +iconName: "get-hot-new-stuff"
> +text: i18nd("plasma_shell_org.kde.plasma.desktop", "Get 
> Wallpaper Plugins...")
> +visible: KAuthorized.authorize("ghns")

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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12041

To: Zren, #plasma
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12041: [WIP] Add "Get Wallpaper Plugins..." button to Config Desktop dialog

2018-04-07 Thread Chris Holland 
Zren created this revision.
Zren added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
Zren requested review of this revision.

REVISION SUMMARY
  Patch 2/2 (other patch is D12040  for 
plasma-workspace)
  
  BUG: #386621
  
  - https://bugs.kde.org/show_bug.cgi?id=386621
  
  Not sure I should be depending on the "image" C++ plugin here.
  
  Screenshot:
  
  F5800267: 2018-04-08___02-19-12.png 
  
  Which opens this GHNS window from Patch 1:
  
  F5800268: 2018-04-08___02-02-49.png 

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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12041

AFFECTED FILES
  desktoppackage/contents/configuration/ConfigurationContainmentAppearance.qml

To: Zren, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart