D12050: Make automounting work even if StorageAccess is ignored

[Nathaniel Graham]

ngraham added a comment.


  In D12050#242890 , @bruns wrote:
  
  > Well, I really do not like what arc is doing behind my back ...
  
  
  It irritated me too at first, but over time I found that it's fairly logical. 
In general, you just need to:
  
  - Always preview with `arc land --preview` before you land, to make sure you 
know what `arc` is going to do
  - Always make a branch; don't diff from master
  - Always branch from the branch that you want to land it on
  - If you change your mind later and decide to land on a different branch, 
re-base the commit prior to landing it: `git rebase --onto origin/ 
master`
  
  I really do recommend having a look at 
https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches.
 I and others have done a lot of work to document the behavior so that using 
`arc` shouldn't feel like a huge headache anymore.

REPOSITORY
  R119 Plasma Desktop

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

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

D12056: Don't have a non-integral size on separators

[Aleix Pol Gonzalez]

apol created this revision.
apol added a reviewer: Kirigami.
Restricted Application added a project: Kirigami.
Restricted Application added a subscriber: plasma-devel.
apol requested review of this revision.

REVISION SUMMARY
  They look odd when placed by a rectangle in a Layout, they will be showing an
  empty space when scaling between the element and the one after.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

AFFECTED FILES
  src/controls/Separator.qml

To: apol, #kirigami
Cc: plasma-devel, apol, davidedmundson, mart, hein

D12050: Make automounting work even if StorageAccess is ignored

[Stefan Brüns]

bruns added a comment.


  Well, I really do not like what arc is doing behind my back ...

REPOSITORY
  R119 Plasma Desktop

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

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

D12050: Make automounting work even if StorageAccess is ignored

[Nathaniel Graham]

ngraham added a comment.


  Well it's too late now, but yes, that would have worked (in general, make 
sure to look before you land by first doing `arc land --preview `). By default, `arc` lands the diff on the parent branch, which in 
this case was master.
  
  Arc documentation is at 
https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches
  
  For now, you can cherry-pick the commit onto the 5.12 branch.

REPOSITORY
  R119 Plasma Desktop

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

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

D12050: Make automounting work even if StorageAccess is ignored

[Stefan Brüns]

bruns added a comment.


  damn, that did not work as expected ... landed on master
  should I just "arc land --onto Plasma/5.12" ?

REPOSITORY
  R119 Plasma Desktop

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

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

D12050: Make automounting work even if StorageAccess is ignored

[Nathaniel Graham]

ngraham added a comment.


  Can you make sure this ends up on the 5.12 branch too?

REPOSITORY
  R119 Plasma Desktop

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

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

D12050: Make automounting work even if StorageAccess is ignored

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:8dedbcdf4197: Make automounting work even if 
StorageAccess is ignored (authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12050?vs=31695=31710

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

AFFECTED FILES
  solid-device-automounter/kded/DeviceAutomounter.cpp

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

D12050: Make automounting work even if StorageAccess is ignored

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Land on the 5.12 branch, please.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fix_automounting_internal

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

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

D12055: Delete border around menubars

[Vlad Zagorodniy]

zzag created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
zzag requested review of this revision.

REPOSITORY
  R98 Breeze for Gtk

BRANCH
  menubar-border

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

AFFECTED FILES
  Breeze-dark-gtk/gtk-3.18/gtk.css
  Breeze-dark-gtk/gtk-3.20/gtk.css
  Breeze-gtk/gtk-3.18/gtk.css
  Breeze-gtk/gtk-3.20/gtk.css

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

D11876: Display star ratings

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R134:19dbc556dbf2: Display star ratings (authored by ngraham).

REPOSITORY
  R134 Discover Software Store

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11876?vs=31697=31704

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

AFFECTED FILES
  discover/qml/ApplicationDelegate.qml
  discover/qml/ApplicationPage.qml
  discover/qml/ApplicationsListPage.qml
  discover/qml/InstalledPage.qml

To: ngraham, apol, #discover_software_store, #vdg
Cc: abetts, rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D11842: Remove categories from app delegates

[Nathaniel Graham]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R134:0911fbcd561c: Remove categories from app delegates 
(authored by ngraham).

REPOSITORY
  R134 Discover Software Store

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11842?vs=31109=31703

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

AFFECTED FILES
  discover/qml/ApplicationDelegate.qml

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

D11876: Display star ratings

[Henrik Fehlauer]

rkflx added a comment.


  In D11876#242783 , @ngraham wrote:
  
  > Thanks for your review, @rkflx. I've fixed the overflow issue on Installed 
page, and in the Compact view more generally (Installed uses the Compact view). 
A by-product of these fixes is that the alignment in general is now fixed as 
well!
  
  
  Umm, this was more of a drive-by comment, did not test properly. Anyway, 
since @apol seems happy, the patch can probably land.

BRANCH
  show-star-ratings (branched from master)

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

To: ngraham, apol, #discover_software_store, #vdg
Cc: abetts, rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[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=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

[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

D12036: Change "different user" icon on login screen

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:7d19c5059b10: Change different user icon on 
login screen (authored by sharvey, committed by ngraham).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12036?vs=31631=31699

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

AFFECTED FILES
  sddm-theme/Main.qml

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

D6183: [Digital Clock] Allow copying current date and time to clipboard

[Bernhard Schiffner]

bschiffner added a comment.


  No, I don't have commit access.
  "Bernhard Schiffner" 
  
  But why my email, Kai-Uwe did 98,7% of the work ...
  
  @Kai-Uwe: Can you do the merge (DigitalClock.qml#77) before pushing, please?

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

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

D12036: Change "different user" icon on login screen

[Nathaniel Graham]

ngraham added a comment.


  Let's stay with `system-switch-user` for now and see how that goes.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  change-user-icon (branched from master)

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

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

D11848: [Kickoff] Reduce hover delay before switching tabs

[Nathaniel Graham]

ngraham added a comment.


  In D11848#242794 , @abetts wrote:
  
  > I am all for reducing the time but maybe keep some lag to tackle the points 
already made in the ticket
  
  
  Yes, this patch reduces it from 250ms to 50ms.

REPOSITORY
  R119 Plasma Desktop

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

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

D11848: [Kickoff] Reduce hover delay before switching tabs

[Andres Betts]

abetts added a comment.


  I am all for reducing the time but maybe keep some lag to tackle the points 
already made in the ticket

REPOSITORY
  R119 Plasma Desktop

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

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

D11848: [Kickoff] Reduce hover delay before switching tabs

[Nathaniel Graham]

ngraham added a comment.


  Friendly ping!

REPOSITORY
  R119 Plasma Desktop

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

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

D11876: Display star ratings

[Nathaniel Graham]

ngraham added a comment.


  Thanks for your review, @rkflx. I've fixed the overflow issue on Installed 
page, and in the Compact view more generally (Installed uses the Compact view). 
A by-product of these fixes is that the alignment in general is now fixed as 
well!

BRANCH
  show-star-ratings (branched from master)

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

To: ngraham, apol, #discover_software_store, #vdg
Cc: abetts, rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D6183: [Digital Clock] Allow copying current date and time to clipboard

[Kai Uwe Broulik]

broulik accepted this revision.
broulik added a comment.
This revision is now accepted and ready to land.


  Let's go for it now! Thanks for patience and apologies it took so long :/
  Do you have commit access? else I need your email address and can push it on 
your behalf.

INLINE COMMENTS

> DigitalClock.qml:77
> +onContextualActionsAboutToShow: {
> +ClipboardMenu.secondsIncluded =main.showSeconds;
> +}

You can merge the two :)

  onContextualActionsAboutToShow: {
  ClipboardMenu.secondsIncluded = main.showSeconds;
  ClipboardMenu.currentDate = main.currentTime;
  }

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

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

D12050: Make automounting work even if StorageAccess is ignored

[Kai Uwe Broulik]

broulik added a comment.


  I did the KF5 port but I don't recall doing that or why it would be there, so 
+1 from me if it fixes that. I don't use automounter so I don't know

REPOSITORY
  R119 Plasma Desktop

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

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

D11876: Display star ratings

[Nathaniel Graham]

ngraham updated this revision to Diff 31697.
ngraham added a comment.


  Also fix the general case for Compact view, which also has the side effect of 
fixing the alignment and centering, yay!

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11876?vs=31694=31697

BRANCH
  show-star-ratings (branched from master)

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

AFFECTED FILES
  discover/qml/ApplicationDelegate.qml
  discover/qml/ApplicationPage.qml
  discover/qml/ApplicationsListPage.qml
  discover/qml/InstalledPage.qml

To: ngraham, apol, #discover_software_store, #vdg
Cc: abetts, rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D12050: Make automounting work even if StorageAccess is ignored

[Stefan Brüns]

bruns added a comment.


  In D12050#242755 , @ngraham wrote:
  
  > Did you mean BUG: 391706?
  
  
  No, this commit *fixes* 389479, but it is a prerequisite for fixing 391706 
(which is essentially a revert, thus would regress 389479).

REPOSITORY
  R119 Plasma Desktop

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

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

D12050: Make automounting work even if StorageAccess is ignored

[Nathaniel Graham]

ngraham added a comment.


  Did you mean BUG: 391706?

REPOSITORY
  R119 Plasma Desktop

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

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

D12050: Make automounting work even if StorageAccess is ignored

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: ngraham.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  There is no reason to make mounting dependent on the StorageAccess.ignore
  flag, which just serves a a hint for visualization.
  
  This change sneaked in when porting to KF5, no justification given.
  
  BUG: 389479

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fix_automounting_internal

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

AFFECTED FILES
  solid-device-automounter/kded/DeviceAutomounter.cpp

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

D11876: Display star ratings

[Nathaniel Graham]

ngraham updated this revision to Diff 31694.
ngraham added a comment.


  Fix the overflow on the Installed page by not showing the stars there (it's 
not relevant on that page)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11876?vs=31646=31694

BRANCH
  show-star-ratings (branched from master)

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

AFFECTED FILES
  discover/qml/ApplicationDelegate.qml
  discover/qml/ApplicationPage.qml
  discover/qml/ApplicationsListPage.qml
  discover/qml/InstalledPage.qml

To: ngraham, apol, #discover_software_store, #vdg
Cc: abetts, rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D3484: Center systemmonitor window properly on multi-screen setup

[Valeriy Malov]

valeriymalov added a comment.


  I've tried to restore old behavior, so:
  
  - If I run KWindowConfig::restoreWindowSize from dialog's constructor, it 
doesn't work on multi-monitor setup very well. windowHandle()→screen() seems to 
be set to leftmost screen (which isn't even primary in my case; is this a 
bug?), so restoreWindowSize loads saved size for same display no matter on 
which display the window is opened. Which makes resizing the dialog on any 
other screen has no effect on the next time it's opened.
  - If I run KWindowConfig::restoreWindowSize in a timer (like it used to be), 
it just messes up window positioning because the window is resized after it's 
been centered by KWin. Probably would mess up KWin rules too.
  
  Patch to demonstrate the issue: F5800781: restore_size.patch 


REPOSITORY
  R120 Plasma Workspace

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

To: valeriymalov, #plasma_workspaces, aacid, mart
Cc: cfeck, davidedmundson, sebas, aacid, graesslin, plasma-devel, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart

D6183: [Digital Clock] Allow copying current date and time to clipboard

[Bernhard Schiffner]

bschiffner updated this revision to Diff 31690.
bschiffner added a comment.


  works for me now / ready to ship
  
  Thanks Kai-Uwe for your advices with the qml problems.
  I'am far from understanding this topic. Mostly cargo cult, but nevertheless 
working.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6183?vs=30975=31690

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

AFFECTED FILES
  applets/digital-clock/package/contents/ui/DigitalClock.qml
  applets/digital-clock/package/contents/ui/main.qml
  applets/digital-clock/plugin/CMakeLists.txt
  applets/digital-clock/plugin/clipboardmenu.cpp
  applets/digital-clock/plugin/clipboardmenu.h
  applets/digital-clock/plugin/digitalclockplugin.cpp

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

D11876: Display star ratings

[Andres Betts]

abetts added a comment.


  +1

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

To: ngraham, apol, #discover_software_store, #vdg
Cc: abetts, rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D12036: Change "different user" icon on login screen

[Andres Betts]

abetts added a comment.


  +1

REPOSITORY
  R120 Plasma Workspace

BRANCH
  change-user-icon (branched from master)

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

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

D11876: Display star ratings

[Henrik Fehlauer]

rkflx added a comment.


  @ngraham Thanks for the version bump. KF 5.45 is not even released yet, but 
after compiling Kirigami master I was able to run Discover with your patch.
  
  FYI, after clicking on Installed there is now some vertical overflowing going 
on.

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

To: ngraham, apol, #discover_software_store, #vdg
Cc: rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12036: Change "different user" icon on login screen

[Scott Harvey]

sharvey added a subscriber: sebas.
sharvey added a comment.


  PS: Credit to @sebas for making Cuttlefish!

REPOSITORY
  R120 Plasma Workspace

BRANCH
  change-user-icon (branched from master)

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

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

D12036: Change "different user" icon on login screen

[Scott Harvey]

sharvey added a comment.


  In D12036#242299 , @ngraham wrote:
  
  >   In the Login screen, it lets you **//find//** other users not listed 
there to log in as.
  
  
  With all due respect to the designers, I never made that connection.
  
  I know the one you selected is named `system-switch-users`, but there's also 
this one, named `system-users`:
  
  F5800737: Screenshot_20180408_17.png 

  
  It doesn't have the arrow (which connotes action), but it clearly 
demonstrates "multiple people", another valid metaphor.
  
  It's simple enough to change if the reviewers like it better.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  change-user-icon (branched from master)

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

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

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

[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

[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

D11876: Display star ratings

[Nathaniel Graham]

ngraham added a comment.


  In D11876#242370 , @rkflx wrote:
  
  > In D11876#241499 , @ngraham 
wrote:
  >
  > > Should we tweak it a bit more?
  >
  >
  > One more thing to look into as a follow-up (besides the font size of the 
title) could be the vertical alignment of the text, i.e. how the first and last 
line relate to the icon, the button and the surrounding box. Currently there is 
neither centering nor aligning of any sorts:
  >
  > F5800277: discover-alignment.png 
  
  
  Yep, it's on my to-do list!

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

To: ngraham, apol, #discover_software_store, #vdg
Cc: rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

[Elvis Angelaccio]

elvisangelaccio added a comment.


  Go for it!

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: arichardson, #plasma, elvisangelaccio
Cc: fvogt, ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

[Alex Richardson]

arichardson added a comment.


  In D4193#242444 , @fvogt wrote:
  
  > What about `Plasma/5.12`? It has a minimum of Qt 5.9 as well.
  
  
  Good point. Any objections against me cherry-picking it to the 5.12 branch?

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: arichardson, #plasma, elvisangelaccio
Cc: fvogt, ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

[Fabian Vogt]

fvogt added a comment.


  What about `Plasma/5.12`? It has a minimum of Qt 5.9 as well.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: arichardson, #plasma, elvisangelaccio
Cc: fvogt, ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

[Alex Richardson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R135:bfd41a95530f: KDEPlatformFileDialog: Fix initial 
directory selection for remote files (authored by arichardson).

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4193?vs=31284=31656

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

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp

To: arichardson, #plasma, elvisangelaccio
Cc: ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D11876: Display star ratings

[Henrik Fehlauer]

rkflx added a comment.


  In D11876#241499 , @ngraham wrote:
  
  > Should we tweak it a bit more?
  
  
  One more thing to look into as a follow-up (besides the font size of the 
title) could be the vertical alignment of the text, i.e. how the first and last 
line relate to the icon, the button and the surrounding box. Currently there is 
neither centering nor aligning of any sorts:
  
  F5800277: discover-alignment.png 

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

To: ngraham, apol, #discover_software_store, #vdg
Cc: rkflx, fabianr, ragreen, acrouthamel, plasma-devel, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

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

[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

D12040: [WIP] Add wallpaperplugin.knsrc + QML function to open GHNS dialog

[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 1/2 (other patch is for plasma-desktop)
  
  BUG: #386621
  
  - https://bugs.kde.org/show_bug.cgi?id=386621
  - To find the `Categories=Plasma Wallpaper Plugin` in the 
`wallpaperplugin.knsrc`, I had to find the `Plasma Wallpaper 
Plugin` on the this API url: 
https://api.kde-look.org/ocs/v1/content/categories
  - To find that url, I looked at: 
https://github.com/KDE/ocs-webserver/blob/master/application/modules/default/Bootstrap.php#L1014
  
  Screenshot of the dialog:
  
  F5800268: 2018-04-08___02-02-49.png 
  
  This screenshot is more releavant to Patch 2 for plasma-desktop:
  
  F5800267: 2018-04-08___02-19-12.png 
  
  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

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  wallpapers/image/CMakeLists.txt
  wallpapers/image/image.cpp
  wallpapers/image/image.h
  wallpapers/image/wallpaperplugin.knsrc

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