D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Nathaniel Graham]

ngraham marked an inline comment as done.
ngraham added a comment.


  ...Gotta make sure that cancel slot actually works, so this is now dependent 
on my patch that fixes it: D12227 

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

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

D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Nathaniel Graham]

ngraham marked 2 inline comments as done.
ngraham added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kdeplatformfiledialoghelper.cpp:108
> Even better to me should be
> 
>   connect(this, ::rejected, m_fileWidget, 
> ::slotCancel);
> 
> PS: You can make a different review to change old syntax (SIGNAL/SLOT) to new 
> function pointers.

Sounds good, I will do that after this!

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

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

D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Nathaniel Graham]

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


  Use a vastly cleaner and cleverer approach (thanks @anthonyfieroni)

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12240?vs=32338=32361

BRANCH
  save-view-config-on-close-and-escape (branched from master)

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

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp

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

D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> kdeplatformfiledialoghelper.cpp:108
>  m_buttons->addButton(m_fileWidget->cancelButton(), 
> QDialogButtonBox::RejectRole);
>  connect(m_buttons, SIGNAL(rejected()), m_fileWidget, SLOT(slotCancel()));
>  connect(m_fileWidget->okButton(), SIGNAL(clicked(bool)), m_fileWidget, 
> SLOT(slotOk()));

Even better to me should be

  connect(this, ::rejected, m_fileWidget, 
::slotCancel);

PS: You can make a different review to change old syntax (SIGNAL/SLOT) to new 
function pointers.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

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

D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> kdeplatformfiledialoghelper.cpp:243
> +{
> +m_fileWidget->writeViewConfig();
> +QDialog::reject();

You can see at line 108 on reject is called slotCancel, so for convenience just 
call it

  m_fileWidget->slotCancel();

Then you can discard D12239 

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added a comment.


  Perhaps we could indeed make use of `QSize::boundedTo()`  to make sure that 
no dimension of the minimum size is ever bigger than a dimension of the current 
screen size. That would help folks like Christoph with his 768x1024 vertical 
screen.
  
  I really think a minimum size of 1024x7somethingssomething represents a big 
improvement in usability here. With a smaller minimum size than this (given 
enough screen space of course), I always found myself endlessly resizing the 
window. With this patch, I now don't. It lends a feeling of polish and 
stability to System Settings that's quite unexpected and welcome.

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> SettingsBase.cpp:128
> +
> +// enforce minimum window size
> +setMinimumSize(SettingsBase::sizeHint());

Why? I use some systems in portrait mode, 768x1024 pixels.

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey updated this revision to Diff 32351.
sharvey added a comment.


  - Add `const`, revise scale factoring for Qt rounding

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12252?vs=32343=32351

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Vlad Zagorodniy]

zzag added inline comments.

INLINE COMMENTS

> SettingsBase.cpp:82
> +// calculate base window size to an appropriate size
> +qreal factor = QGuiApplication::primaryScreen()->devicePixelRatio();
> +return QSize(1024*factor, 768*factor);

You forgot `const`.

Also, you could multiply whole QSize by `factor` so Qt will round width and 
height. ;-)

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey updated this revision to Diff 32343.
sharvey added a comment.


  - Revert back to 700 for Y scale

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12252?vs=32341=32343

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added a comment.


  In D12252#247757 , @zzag wrote:
  
  > These defaults are really big, especially height. What about folks with 
1366x768 screen resolution? 
http://gs.statcounter.com/screen-resolution-stats/desktop/worldwide
  
  
  The issue is that the KCMs themselves mostly look good at a minimum size of 
1024x768. There's unfortunately such variation in their designs that some look 
fine at much smaller size, and a very small number require something even 
better, but for the most part, I think we should choose a sensible default. For 
people with 1336x768 screens, it's not like allowing the window to be smaller 
is going to be very useful to them anyway. That screen resolution is 
sufficiently small and low resolution that in my experience, people using it 
just maximize everything anyway.

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added a comment.


  KWin adds a titlebar to the chosen window size that is itself of variable 
height depending on the titlebar font size. So if we want to maintain 
compatibility with an actual 1024x768 display, we'll need to at reduce our 
minimum height by 30 px or so--possibly more. Might be saner to just to back 
down to 700.

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Vlad Zagorodniy]

zzag added a comment.


  These defaults are really big, especially height. What about folks with 
1366x768 screen resolution? 
http://gs.statcounter.com/screen-resolution-stats/desktop/worldwide

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey added a comment.


  > Heh, that wasn't a request targeted at you, but rather a question for 
others. :)
  
  Should I back those 68 pixels back out (for now?)

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added a comment.


  In D12252#247750 , @sharvey wrote:
  
  > In D12252#247745 , @ngraham 
wrote:
  >
  > > Ah, so it is! Any objections to increasing the effective minimum size to 
1024x768? It seems that this is the size that the KCMs were actually designed 
for, so it makes sense to me as a sensible default, but let's collect some more 
opinions first.
  >
  >
  > 68 pixels added.
  
  
  Heh, that wasn't a request targeted at you, but rather a question for others. 
:) This patch will make System Settings effectively unusable for screens 
smaller than 1024x768 (it's currently //technically// usable, but not in 
practice since you need to scroll nearly every KCM to see anything; it's a 
really bad user experience). My vote is a yes since the KCMs are themselves 
mostly unusable at smaller sizes, but we should still ask first.

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey added a comment.


  In D12252#247745 , @ngraham wrote:
  
  > Ah, so it is! Any objections to increasing the effective minimum size to 
1024x768? It seems that this is the size that the KCMs were actually designed 
for, so it makes sense to me as a sensible default, but let's collect some more 
opinions first.
  
  
  68 pixels added.

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey updated this revision to Diff 32341.
sharvey added a comment.


  - Tweak size to 1024x768

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12252?vs=32337=32341

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

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


  Ah, so it is! Any objections to increasing the effective minimum size to 
1024x768? It seems that this is the size that the KCMs were actually designed 
for, so it makes sense to me as a sensible default, but let's collect some more 
opinions first.

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey added a comment.


  > All of this is already implemented and works as expected if you simply 
remove the `resize()` call. What needs to be done is to specify the default 
size without always calling `resize()`. Either that, or enforce 1024x700 as the 
minimum.
  
  Well then it's already implemented! We're only calling `setMinimumSize()` 
now; no longer using `resize()`.
  
  (I just discovered that this code was already there as I went looking around 
to see where it would belong... and it's already there!)

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added a comment.


  In D12252#247719 , @sharvey wrote:
  
  > I thought about that. I figured there wasn't any harm in letting the user 
resize the window even if it wouldn't be saved.
  
  
  User-chosen window sizes should //always// be saved. The only alternative is 
to make the window non-resizable in the first place.
  
  Do we want to go with the standard routine of saving the window geometry and 
restoring it the next time the user launches the application? I can work that 
in, maybe with a `firstLaunch` boolean flag that only triggers the manual 
setting of the window size on the very first run. The window still won't get 
smaller than `minimumSize`, but if someone wants to enlarge it, we could save 
that size and restore it in the future.
  
  All of this is already implemented and works as expected if you simply remove 
the `resize()` call. What needs to be done is to specify the default size 
without always calling `resize()`. Either that, or enforce 1024x700 as the 
minimum.

REPOSITORY
  R124 System Settings

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

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

D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Nathaniel Graham]

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


  Fix typo

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12240?vs=32236=32338

BRANCH
  save-view-config-on-close-and-escape (branched from master)

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

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp
  src/platformtheme/kdeplatformfiledialoghelper.h

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey added a comment.


  > Once that's fixed, there's still one more issue here: always calling 
`resize()` this means that once the user resizes the window, that custom size 
will be overridden on the next launch. We want to set a better default size, 
not enforce a mandatory size. I wonder if we should just remove the `resize()` 
call and change `setMinimumSize()` to enforce 1024x700? Most KCMs look pretty 
bad at smaller sizes anyway.
  
  I thought about that. I figured there wasn't any harm in letting the user 
resize the window even if it wouldn't be saved. Do we want to go with the 
standard routine of saving the window geometry and restoring it the next time 
the user launches the application? I can work that in, maybe with a 
`firstLaunch` boolean flag that only triggers the manual setting of the window 
size on the very first run. The window still won't get smaller than 
`minimumSize`, but if someone wants to enlarge it, we could save that size and 
restore it in the future.

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey updated this revision to Diff 32337.
sharvey added a comment.


  - Remove qBound and division by 96 for pixel ratio

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12252?vs=32336=32337

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> SettingsBase.cpp:82
> +// calculate base window size to an appropriate size
> +qreal factor = qBound(1.0, 
> QGuiApplication::primaryScreen()->devicePixelRatio()/96., 3.0);
> +return QSize(1024*factor, 700*factor);

you don't need to divide by 96 anymore; `devicePixelRatio()` gives you a number 
generally between 1 and 4, depending on the scale factor used. In fact, you 
might be able to remove the entire `qBounds` part too, since that won't be 
forwards-compatible with the next decade's 16k displays...

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey updated this revision to Diff 32336.
sharvey added a comment.


  - Use `devicePixelRatio` for base size instead of `physicalDotsPerInch`

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12252?vs=32323=32336

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added a comment.


  I think I found the problem: `physicalDotsPerInch()` is not the correct 
property to be checking here, as it will vary depending on the display's pixel 
density. You want to change that to check  `devicePixelRatio()` instead. That 
gives me perfect results.
  
  Once that's fixed, there's still one more issue here: always calling 
`resize()` this means that once the user resizes the window, that custom size 
will be overridden on the next launch. We want to set a better default size, 
not enforce a mandatory size. I wonder if we should just remove the `resize()` 
call and change `setMinimumSize()` to enforce 1024x700? Most KCMs look pretty 
bad at smaller sizes anyway.

REPOSITORY
  R124 System Settings

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

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

D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> kdeplatformfiledialoghelper.cpp:240
> +// Save the view settings before closing the dialog when the Escape key
> +// is presed or the close button in the titlebar is clicked
> +void KDEPlatformFileDialog::reject()

typo: pressed

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

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

D12263: [kstyle] don't suppress deprecation warnings

[Vlad Zagorodniy]

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

REPOSITORY
  R31 Breeze

BRANCH
  dont-suppress-deprecated-warnings

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

AFFECTED FILES
  kstyle/CMakeLists.txt

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

D12261: [kstyle] drop QStyleOptionFrameV3 in Qt 5 style plugin

[Vlad Zagorodniy]

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

REPOSITORY
  R31 Breeze

BRANCH
  drop-QStyleOptionFrameV3

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

AFFECTED FILES
  kstyle/breezestyle.cpp

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

D12262: [kstyle] drop QStyleOptionDockWidgetV2 in Qt 5 style plugin

[Vlad Zagorodniy]

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

REPOSITORY
  R31 Breeze

BRANCH
  drop-QStyleOptionDockWidgetV2

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

AFFECTED FILES
  kstyle/breezestyle.cpp

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

D12260: [kstyle] drop QStyleOptionFrameV2 in Qt 5 style plugin

[Vlad Zagorodniy]

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

REPOSITORY
  R31 Breeze

BRANCH
  drop-QStyleOptionFrameV2

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

AFFECTED FILES
  kstyle/breezestyle.cpp

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

D12259: [kstyle] drop QStyleOptionViewItemV4 in Qt 5 style plugin

[Vlad Zagorodniy]

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

REPOSITORY
  R31 Breeze

BRANCH
  drop-QStyleOptionViewItemV4

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

AFFECTED FILES
  kstyle/breezestyle.cpp

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

D12258: [kstyle] drop QStyleOptionTabWidgetFrameV2 in Qt 5 style plugin

[Vlad Zagorodniy]

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

REPOSITORY
  R31 Breeze

BRANCH
  drop-QStyleOptionTabWidgetFrameV2

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

AFFECTED FILES
  kstyle/breezestyle.cpp

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

D12257: [kstyle] drop QStyleOptionTabV3 in Qt 5 style plugin

[Vlad Zagorodniy]

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

REPOSITORY
  R31 Breeze

BRANCH
  drop-QStyleOptionTabV3

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

AFFECTED FILES
  kstyle/breezestyle.cpp

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

D12256: [kstyle] drop QStyleOptionProgressBarV2 in Qt 5 style plugin

[Vlad Zagorodniy]

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

REVISION SUMMARY
  D11566  is now in KDE Frameworks 5.45 so 
we can drop
  QStyleOptionProgressBarV2.

REPOSITORY
  R31 Breeze

BRANCH
  drop-QStyleOptionProgressBarV2

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

AFFECTED FILES
  kstyle/breezestyle.cpp

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey updated this revision to Diff 32323.
sharvey added a comment.


  - Adjust Y scale to 700

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12252?vs=32314=32323

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey added a comment.


  In D12252#247567 , @ngraham wrote:
  
  > Hmm, that's not the behavior I see at all. For me, `factor` appears to be 1 
with not running a HiDPi scale factor, so I get an actual size that's 
essentially equal to the number it's multiplied by. (e.g. `return 
QSize(1024*factor, 700*factor)` yields window content that's actually 1024x700 
). If I run with `QT_SCALE_FACTOR=1.3 systemsettings5`, everything is scaled 
perfectly. Do you have some wonky DPI settings on your test box or something?
  
  
  While wonky things do frequently happen when I'm around, this doesn't seem to 
be the case. KInfoCenter reports a 96x96DPI display. I can't guarantee this is 
the highest-grade display on the market, though.
  
  F5809984: Screenshot_20180416_142747.png 

  
  I'll leave it at 700 rather than hack away at some weird workaround.

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added a comment.


  Hmm, that's not the behavior I see at all. For me, `factor` appears to be 1 
with not running a HiDPi scale factor, so I get an actual size that's 
essentially equal to the number it's multiplied by. (e.g. `return 
QSize(1024*factor, 700*factor)` yields window content that's actually 1024x700 
). If I run with `QT_SCALE_FACTOR=1.3 systemsettings5`, everything is scaled 
perfectly. Do you have some wonky DPI settings on your test box or something?

REPOSITORY
  R124 System Settings

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

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

KDE CI: Plasma plasma-vault kf5-qt5 FreeBSDQt5.9 - Build # 20 - Still Failing!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Plasma%20plasma-vault%20kf5-qt5%20FreeBSDQt5.9/20/
 Project:
Plasma plasma-vault kf5-qt5 FreeBSDQt5.9
 Date of build:
Mon, 16 Apr 2018 18:13:43 +
 Build duration:
43 sec and counting
   CONSOLE OUTPUT
  [...truncated 62.68 KB...]JOB_NAME  = 'Plasma plasma-vault kf5-qt5 FreeBSDQt5.9'BUILD_TAG = 'jenkins-Plasma plasma-vault kf5-qt5 FreeBSDQt5.9-20'BUILD_NUMBER  = '20'JOB_BASE_NAME = 'Plasma plasma-vault kf5-qt5 FreeBSDQt5.9'XDG_CONFIG_DIRS   = '/usr/home/jenkins/install-prefix/etc/xdg:/usr/local/etc/xdg'STAGE_NAME= 'Configuring Build'BUILD_URL = 'https://build.kde.org/job/Plasma%20plasma-vault%20kf5-qt5%20FreeBSDQt5.9/20/'_ = '/usr/local/bin/java'HOME  = '/home/jenkins'LD_LIBRARY_PATH   = '/usr/home/jenkins/install-prefix/lib'NODE_LABELS   = 'FreeBSD Builder 1 FreeBSDQt5.9'JENKINS_URL   = 'https://build.kde.org/'PKG_CONFIG_PATH   = '/usr/home/jenkins/install-prefix/lib/pkgconfig:/usr/libdata/pkgconfig:/usr/local/libdata/pkgconfig'QT_SELECT = 'qt5'LOGNAME   = 'jenkins'OLDPWD= '/usr/home/jenkins'JOB_DISPLAY_URL   = 'https://build.kde.org/job/Plasma%20plasma-vault%20kf5-qt5%20FreeBSDQt5.9/display/redirect'CMAKE_PREFIX_PATH = '/usr/home/jenkins/install-prefix:/usr/:/usr/local/'BUILD_ID  = '20'MAIL  = '/var/mail/jenkins'XDG_CURRENT_DESKTOP   = 'KDE'EXECUTOR_NUMBER   = '0'RUN_CHANGES_DISPLAY_URL   = 'https://build.kde.org/job/Plasma%20plasma-vault%20kf5-qt5%20FreeBSDQt5.9/20/display/redirect?page=changes'HUDSON_HOME   = '/home/production/data'JENKINS_NODE_COOKIE   = 'c7ec9312-2c4d-4d6b-9686-dcf63c36b91e'QML2_IMPORT_PATH  = '/usr/home/jenkins/install-prefix/lib/qml'XDG_DATA_DIRS = '/usr/home/jenkins/install-prefix/share:/usr/share:/usr/local/share'BUILD_DISPLAY_NAME= '#20'JOB_URL   = 'https://build.kde.org/job/Plasma%20plasma-vault%20kf5-qt5%20FreeBSDQt5.9/'HUDSON_URL= 'https://build.kde.org/'PWD   = '/usr/home/jenkins/workspace/Plasma plasma-vault kf5-qt5 FreeBSDQt5.9'RUN_DISPLAY_URL   = 'https://build.kde.org/job/Plasma%20plasma-vault%20kf5-qt5%20FreeBSDQt5.9/20/display/redirect'QT_PLUGIN_PATH= '/usr/home/jenkins/install-prefix/lib/plugins'PATH  = '/usr/home/jenkins/install-prefix/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/home/jenkins/bin'MM_CHARSET= 'UTF-8'TERM  = 'su'SHELL = '/usr/local/bin/bash'LANG  = 'en_US.UTF-8'USER  = 'jenkins'JAVA_HOME = '/usr/local/openjdk8-jre'BLOCKSIZE = 'K'== Commencing Configuration:cmake -DCMAKE_BUILD_TYPE=Debug -DECM_ENABLE_SANITIZERS='address' -DBUILD_TESTING=ON -DCMAKE_INSTALL_PREFIX="/home/jenkins/install-prefix/"  "/usr/home/jenkins/workspace/Plasma plasma-vault kf5-qt5 FreeBSDQt5.9"-- The C compiler identification is Clang 4.0.0-- The CXX compiler identification is Clang 4.0.0-- Check for working C compiler: /usr/bin/cc-- Check for working C compiler: /usr/bin/cc -- works-- Detecting C compiler ABI info-- Detecting C compiler ABI info - done-- Detecting C compile features-- Detecting C compile features - done-- Check for working CXX compiler: /usr/bin/c++-- Check for working CXX compiler: /usr/bin/c++ -- works-- Detecting CXX compiler ABI info-- Detecting CXX compiler ABI info - done-- Detecting CXX compile features-- Detecting CXX compile features - done-- -- Could not set up the appstream test. appstreamcli is missing.-- Looking for __GLIBC__-- Looking for __GLIBC__ - not found-- Performing Test _OFFT_IS_64BIT-- Performing Test _OFFT_IS_64BIT - Success-- Performing Test HAVE_DATE_TIME-- Performing Test HAVE_DATE_TIME - Success-- Found KF5Activities: /usr/home/jenkins/install-prefix/lib/cmake/KF5Activities/KF5ActivitiesConfig.cmake (found version "5.45.0") -- Found KF5Config: /usr/home/jenkins/install-prefix/lib/cmake/KF5Config/KF5ConfigConfig.cmake (found version "5.45.0") -- Found KF5ConfigWidgets: /usr/home/jenkins/install-prefix/lib/cmake/KF5ConfigWidgets/KF5ConfigWidgetsConfig.cmake (found version "5.45.0") -- Found KF5CoreAddons: /usr/home/jenkins/install-prefix/lib/cmake/KF5CoreAddons/KF5CoreAddonsConfig.cmake (found version "5.45.0") -- Found KF5DBusAddons: /usr/home/jenkins/install-prefix/lib/cmake/KF5DBusAddons/KF5DBusAddonsConfig.cmake (found version "5.45.0") -- Found Gettext: /usr/local/bin/msgmerge (found version "0.19.8.1") -- Found KF5I18n: /usr/home/jenkins/install-prefix/lib/cmake/KF5I18n/KF5I18nConfig.cmake (found version "5.45.0") -- Found KF5KIO: 

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey added a comment.


  In D12252#247563 , @ngraham wrote:
  
  > Much better! I think I'd still prefer 700 for the height, since at 600, the 
following KCMs have vertical scrollbars, but lose them at 700:
  
  
  700 comes out a bit too large for me (see very bottom). Maybe Vlad is right 
and I need to run this through a better bounding function?
  F5809937: Screenshot_20180416_130509.png 


REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Nathaniel Graham]

ngraham added a comment.


  Much better! I think I'd still prefer 700 for the height, since at 600, the 
following KCMs have vertical scrollbars, but lose them at 700:
  
  - Fonts
  - Screen Edges
  - Window Behavior
  - KDE Wallet
  - File Associations
  - Network
  - Cookies
  - Keyboard
  - Energy Saving

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to determine minimum window width.

[Scott Harvey]

sharvey marked an inline comment as done.
sharvey added a comment.


  In D12252#247508 , @zzag wrote:
  
  > What if sizeHint() returns a size which is bigger than screen size? Please 
use `QSize::boundedTo`.
  
  
  Can that happen, if we're basing `sizeHint` off the screen's physical DPI?

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge window size: wide enough to show both sidebarsUse (adjusted) calculation in `SizeHint` to determine minimum window width to display both sidebars. Manually apply size with call to `res

[Scott Harvey]

sharvey marked an inline comment as done.
sharvey added inline comments.

INLINE COMMENTS

> ngraham wrote in SettingsBase.cpp:159
> Don't remove this, or else it becomes possible to resize the window to 
> absurdly, unusably small sizes. The value could be changed, but we shouldn't 
> remove this entirely.

Moved to line 130; keep together with initial window size setting. I went with 
an arbitrary but common 800x600 for the minimum size.

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge window size: wide enough to show both sidebarsUse (adjusted) calculation in `SizeHint` to determine minimum window width to display both sidebars. Manually apply size with call to `res

[Scott Harvey]

sharvey updated this revision to Diff 32314.
sharvey added a comment.


  - Adjust calculation; re-add `minimumSize()` property
  
  Updating D12252: Enlarge window size: wide enough to show both sidebars
  ===
  
  Use (adjusted) calculation in `SizeHint` to determine minimum window width to 
display both sidebars. Manually apply size with call to `resize()`

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12252?vs=32306=32314

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12248: [Desktop Theme KCM] Port to new design

[Nathaniel Graham]

ngraham added a comment.


  This looks really good.

REPOSITORY
  R119 Plasma Desktop

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

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

D12252: Enlarge window size: wide enough to show both sidebarsUse (adjusted) calculation in `SizeHint` to determine minimum window width to display both sidebars. Manually apply size with call to `res

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> SettingsBase.cpp:159
>  helpActionMenu->setMenu( dynamic_cast( 
> factory()->container(QStringLiteral("help"), this) ) );
> -setMinimumSize(620,430);
>  toolBar()->setMovable(false); // We don't allow any changes

Don't remove this, or else it becomes possible to resize the window to 
absurdly, unusably small sizes. The value could be changed, but we shouldn't 
remove this entirely.

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge window size: wide enough to show both sidebarsUse (adjusted) calculation in `SizeHint` to determine minimum window width to display both sidebars. Manually apply size with call to `res

[Nathaniel Graham]

ngraham added a comment.


  Tested it out! The patch succeeds at altering the default size, but I'm not 
sure it accomplishes either of the goals of opening large enough to make the 
home screen and most KCMs look good, nor your intention of making it wide 
enough to always show both columns (not sure if we should shoot for that, 
either: Plasma should still be usable on a 1024x768 screen).
  
  My recommendation would be to forget about opening to a size that shows both 
columns, and focus on opening to a large enough default size that the home 
screen and most KCMs look good. In my testing, `QSize(1020*factor, 700*factor)` 
opens to a much more comfortable size for me that results in nearly all KCMs 
presented without any scrollbars. It doesn't seem like a coincidence that this 
is very close to 1024x768; almost as if the KCMs were designed for that size in 
the first place...
  
  With that change, this patch could be marked as fixing 
https://bugs.kde.org/show_bug.cgi?id=389617

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge window size: wide enough to show both sidebarsUse (adjusted) calculation in `SizeHint` to determine minimum window width to display both sidebars. Manually apply size with call to `res

[Vlad Zagorodniy]

zzag added a comment.


  What if sizeHint() returns a size which is bigger than screen size? Please 
use `QSize::boundedTo`.

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge window size: wide enough to show both sidebarsUse (adjusted) calculation in `SizeHint` to determine minimum window width to display both sidebars. Manually apply size with call to `res

[Scott Harvey]

sharvey updated this revision to Diff 32306.
sharvey added a comment.


  - Uncomment line left from testing
  
  Updating D12252: Enlarge window size: wide enough to show both sidebars
  ===
  
  Use (adjusted) calculation in `SizeHint` to determine minimum window width to 
display both sidebars. Manually apply size with call to `resize()`

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12252?vs=32304=32306

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12252: Enlarge window size: wide enough to show both sidebarsUse (adjusted) calculation in `SizeHint` to determine minimum window width to display both sidebars. Manually apply size with call to `res

[Scott Harvey]

sharvey added a comment.


  The calculation is aiming for a minimum width of 1200px, which seems to be 
the minimum size needed to display both sidebars. Since the original 
calculation uses `physicalDotsPerInch`, I'm only able to use my own laptop as a 
test subject. It's a 15" 1920x1080 screen. Checking `physicalDotsPerInch` gives 
me a value of ~1.47. That's how I derived the value of 815 to multiply by. The 
`qBound` function should keep the size to something reasonable, regardless of 
the screen size.
  
  I don't have another machine to test on (particularly not a HiDPI screen), so 
this is going to need a test or two. It's possible that `physicalDotsPerInch` 
may not be the ideal base for the calculation, in which case some more advanced 
algebra may be required.

REPOSITORY
  R124 System Settings

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

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

D12252: Enlarge window size: wide enough to show both sidebarsUse (adjusted) calculation in `SizeHint` to determine minimum window width to display both sidebars. Manually apply size with call to `res

[Scott Harvey]

sharvey created this revision.
sharvey added reviewers: ngraham, mart.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
sharvey requested review of this revision.

TEST PLAN
  - Compile systemsettings with path
- Launch system settings (systemsettings5)
- Determine if window is wide enough to display both sidebars at once
- Close System Settings and reoopen; ensure size remains set

REPOSITORY
  R124 System Settings

BRANCH
  enlarge-default-size (branched from master)

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

AFFECTED FILES
  app/SettingsBase.cpp

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

D12055: Remove border around menubars

[Vlad Zagorodniy]

zzag reclaimed this revision.
zzag added a comment.


  Oh, I misunderstood you.

REPOSITORY
  R98 Breeze for Gtk

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

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

D12248: [Desktop Theme KCM] Port to new design

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Plasma, VDG.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  The preview code could luckily be left completely untouched.

TEST PLAN
  - Verified that current index is correct upon opening
  - Verified that changing themes works
  - Verified that "open from file" works
  - Verified that GHNS works
  - Verified that theme removal works (is now animated, the delegate zooms out 
like a plasmoid on the desktop) and then the other items move ot fill the gap
  
  F5809788: Screenshot_20180416_174837.png 

  In case of error I used the new `InlineMessage` (before it would just show a 
small label next to the buttons)
  F5809789: Screenshot_20180416_175219.png 


REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/main.qml

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

D12088: port the ksplash kcm to the new design

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:a6761883c8e7: port the ksplash kcm to the new design 
(authored by mart).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12088?vs=31958=32295

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

AFFECTED FILES
  kcms/ksplash/kcm.cpp
  kcms/ksplash/kcm.h
  kcms/ksplash/package/contents/ui/main.qml

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

D12148: update the wallpaper dialog to the new design

[Marco Martin]

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 R120:c66e998d7b6c: update the wallpaper dialog to the new 
design (authored by mart).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12148?vs=32289=32294

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

AFFECTED FILES
  wallpapers/image/imagepackage/contents/ui/WallpaperDelegate.qml
  wallpapers/image/imagepackage/contents/ui/config.qml

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

D12090: port the look and feel kcm to new design

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:0b91f62e6920: port the look and feel kcm to new design 
(authored by mart).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12090?vs=32282=32293

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

AFFECTED FILES
  kcms/lookandfeel/kcm.cpp
  kcms/lookandfeel/kcm.h
  kcms/lookandfeel/package/contents/ui/main.qml

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

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

[Andres Betts]

abetts added a comment.


  In D12041#247368 , @Zren wrote:
  
  > In D12041#247352 , @abetts 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!

REPOSITORY
  R119 Plasma Desktop

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

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

D12148: update the wallpaper dialog to the new design

[Marco Martin]

mart updated this revision to Diff 32289.
mart added a comment.


  - remove explicit width

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12148?vs=32281=32289

BRANCH
  phab/wallpaperdialredesign

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

AFFECTED FILES
  wallpapers/image/imagepackage/contents/ui/WallpaperDelegate.qml
  wallpapers/image/imagepackage/contents/ui/config.qml

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

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

[Chris Holland]

Zren added a comment.


  In D12041#247352 , @abetts 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.

REPOSITORY
  R119 Plasma Desktop

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

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

D12146: [Window List] Show pin icon only when in panel

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:eaa5b4364f8b: [Window List] Show pin icon only when in 
panel (authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12146?vs=31967=32288

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

AFFECTED FILES
  applets/window-list/contents/ui/main.qml

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

D12146: [Window List] Show pin icon only when in panel

[Marco Martin]

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


  good idea

REPOSITORY
  R119 Plasma Desktop

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

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

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

[Andres Betts]

abetts added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

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

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

D12148: update the wallpaper dialog to the new design

[David Edmundson]

davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> config.qml:286-287
> +id: wallpapersGrid
>  anchors.fill: parent
> -
> -frameVisible: true
> -highlightOnFocus: true;
> -
> -Component.onCompleted: {
> -//replace the current binding on the scrollbar that makes it 
> visible when content doesn't fit
> -
> -//otherwise we adjust gridSize when we hide the vertical 
> scrollbar and
> -//due to layouting that can make everything adjust which 
> changes the contentWidth/height which
> -//changes our scrollbars and we continue being stuck in a 
> loop
> -
> -//looks better to not have everything resize anyway.
> -//BUG: 336301
> -__verticalScrollBar.visible = true
> -}
> -
> -GridView {
> -id: wallpapersGrid
> -model: imageWallpaper.wallpaperModel
> -currentIndex: -1
> -focus: true
> -
> -cellWidth: Math.floor(wallpapersGrid.width / 
> Math.max(Math.floor(wallpapersGrid.width / (units.gridUnit*12)), 3))
> -cellHeight: Math.round(cellWidth / 
> (imageWallpaper.targetSize.width / imageWallpaper.targetSize.height))
> -
> -anchors.margins: 4
> -boundsBehavior: Flickable.StopAtBounds
> -
> -delegate: WallpaperDelegate {
> -color: cfg_Color
> -}
> -
> -onContentHeightChanged: {
> -wallpapersGrid.currentIndex = 
> imageWallpaper.wallpaperModel.indexOf(cfg_Image);
> -
> wallpapersGrid.positionViewAtIndex(wallpapersGrid.currentIndex, 
> GridView.Visible)
> -}
> -
> -Keys.onPressed: {
> -if (count < 1) {
> -return;
> -}
> -
> -if (event.key == Qt.Key_Home) {
> -currentIndex = 0;
> -} else if (event.key == Qt.Key_End) {
> -currentIndex = count - 1;
> -}
> -}
> -
> -Keys.onLeftPressed: moveCurrentIndexLeft()
> -Keys.onRightPressed: moveCurrentIndexRight()
> -Keys.onUpPressed: moveCurrentIndexUp()
> -Keys.onDownPressed: moveCurrentIndexDown()
> -
> -Connections {
> -target: imageWallpaper
> -onCustomWallpaperPicked: {
> -wallpapersGrid.currentIndex = 0;
> -cfg_Image = path;
> -}
> -}
> -
> +width: Math.min(implicitWidth, parent.width)
> +

You can't have both.

REPOSITORY
  R120 Plasma Workspace

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

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

D12148: update the wallpaper dialog to the new design

[Andres Betts]

abetts added a comment.


  When you can, can you share a screenshot of the progress? I am so excited to 
see this!

REPOSITORY
  R120 Plasma Workspace

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

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

D12117: handle in the top toolbar

[Marco Martin]

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 R169:a14d55fc9222: handle in the top toolbar (authored by 
mart).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D12117?vs=31896=32286#toc

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12117?vs=31896=32286

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

AFFECTED FILES
  examples/gallerydata/contents/ui/gallery/MultipleColumnsGallery.qml
  examples/simpleexamples/simpleChatApp.qml
  kirigami.qrc
  src/controls/ContextDrawer.qml
  src/controls/OverlayDrawer.qml
  src/controls/Separator.qml
  src/controls/ToolBarApplicationHeader.qml
  src/controls/templates/ApplicationHeader.qml
  src/controls/templates/InlineMessage.qml
  src/controls/templates/OverlayDrawer.qml
  src/controls/templates/private/BackButton.qml
  src/controls/templates/private/ContextIcon.qml
  src/controls/templates/private/ForwardButton.qml
  src/controls/templates/private/GenericDrawerIcon.qml
  src/controls/templates/private/MenuIcon.qml
  src/styles/org.kde.desktop/OverlayDrawer.qml

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

D12090: port the look and feel kcm to new design

[Marco Martin]

mart updated this revision to Diff 32282.
mart added a comment.


  - remove dead code

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12090?vs=31959=32282

BRANCH
  phab/lookandfeelredesign

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

AFFECTED FILES
  kcms/lookandfeel/kcm.cpp
  kcms/lookandfeel/kcm.h
  kcms/lookandfeel/package/contents/ui/main.qml

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

D12148: update the wallpaper dialog to the new design

[Marco Martin]

mart updated this revision to Diff 32281.
mart added a comment.


  - adapt to new design

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12148?vs=31970=32281

BRANCH
  phab/wallpaperdialredesign

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

AFFECTED FILES
  wallpapers/image/imagepackage/contents/ui/WallpaperDelegate.qml
  wallpapers/image/imagepackage/contents/ui/config.qml

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

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

[Chris Holland]

Zren added inline comments.

INLINE COMMENTS

> davidedmundson wrote in wallpaperplugin.cpp:26
> You don't need to do this.
> 
> QPointer has a custom -> operator
> 
> So m_newStuffDialog->setTitle works just fine.
> 
> Same for in newStuffFinished

I copy pasted from `image.cpp` ... which apparently hasn't been touched in 4 
years since the import from kde-workspace.

- 
https://github.com/KDE/plasma-workspace/blame/master/wallpapers/image/image.cpp#L600

Will that function need updating too?

REPOSITORY
  R131 Plasma Wallpapers

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

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

Minutes meeting week 16

[Bhushan Shah]

logs and notes!

Announcement: Since we will be meeting in-person next week, there will
be no meeting on the 23rd. Also since I will reach home on 30th I will
need someone to take care of meeting on the week 18.

-- 
Bhushan Shah
http://blog.bshah.in
IRC Nick : bshah on Freenode
GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D
2018-04-16 15:30:12  notmart_: kbroulik: d_ed: romangg : meeting!
2018-04-16 15:30:15   here :)
2018-04-16 15:30:17   here
2018-04-16 15:31:12  alright, notmart_ starts
2018-04-16 15:31:24   * kirigami bugfixes
2018-04-16 15:31:25   ** fixed bottom drawer behavior
2018-04-16 15:31:27   ** layers anymation different from 
pages' one
2018-04-16 15:31:28   * work on thetophandle branch of 
kirigami: desktop apps more desktop looking
2018-04-16 15:31:30   * port look and feel and ksplash kcms 
to the new design, big code simplification
2018-04-16 15:31:31   * port the plasma wallpaper dialog to 
the same design
2018-04-16 15:31:50   this week i have to finish that corona 
refactor, 
2018-04-16 15:31:59   o/
2018-04-16 15:32:00   layers anjimation?
2018-04-16 15:32:21   finish those kcm redesigns and 
resurrect my tabletbode stuff as i want to show it at the sprint
2018-04-16 15:32:42here
2018-04-16 15:33:02  if no question for notmart_ then kbroulik turn
2018-04-16 15:33:07   kbroulik: in kirigami that there are 2 
separate concepts for push/pop page, one that is called pagerow and is the 
main, another one as layers more intended to be modal, so now pages animate 
horizontally, layers vertically
2018-04-16 15:33:13   aanyways, done
2018-04-16 15:33:14   I see
2018-04-16 15:33:17   * Plasma-pa now shows OSD when default 
output changes, e.g. because you (un)plugged headphones or paired a Bluetooth 
audio device
2018-04-16 15:33:21   * OSD also enforces plain text now
2018-04-16 15:33:21   * Fixed image wallpaper blur
2018-04-16 15:33:25   * Kickoff now supports non-square icons 
in panel, code borrowed from Kicker
2018-04-16 15:33:28   * Did some Plasma profiling
2018-04-16 15:33:28   ** Optimized WorkspaceScripting, no 
longer calls QFontMetrics for gridUnit when nobody calls it (only does on setup 
or panel creation)
2018-04-16 15:33:35   * Performance optimizations to the file 
dialog
2018-04-16 15:33:38   * Tried to fix "Print" option missing 
from Okular part in Ark (e.g. opening a PDF from Ark)
2018-04-16 15:33:38   ** Made D12067 but bug lies elsewhere 
since works in Konqueror afaict
2018-04-16 15:33:38   Diff 12067 "[Okular Part] Add "Print" 
action" [Changes Planned] https://phabricator.kde.org/D12067
2018-04-16 15:33:41   * Generic bug fixing and triaging and 
code reviewing
2018-04-16 15:33:41   * Please check out: D12146
2018-04-16 15:33:41   Diff 12146 "[Window List] Show pin icon 
only when in panel" [Needs Review] https://phabricator.kde.org/D12146
2018-04-16 15:33:53   questions || next
2018-04-16 15:34:09  d_ed: your turn
2018-04-16 15:34:53   most exciting thing was finishing the XdgOutput 
protocol
2018-04-16 15:35:04   which sends a rectangle
2018-04-16 15:35:11   and then someone else gets a rectangle
2018-04-16 15:35:42   is for multiscreen?
2018-04-16 15:35:46   it's for scaling
2018-04-16 15:35:57   ah, right
2018-04-16 15:36:17   the names of wayland extensions are 
getting seriously confusing with all that stuff named almost the same :/
2018-04-16 15:36:26   xwayland does something mental for legacy 
reasons, so this new protocol is needed so that it knows whether to do 
something weird or not
2018-04-16 15:36:38   also allows for fractional scaling in future 
(with some other mods elsewhere)
2018-04-16 15:36:48   you need to sound more excited about 
Rectangkles
2018-04-16 15:37:09   so, we're not even 10 years in with 
Wayland, let alone widespread adoption, and already have to fight legacy shit. 
brilliant :)
2018-04-16 15:37:29   oh, and I fixed that crash when changing from 
desktop to folder view in plasma-framework that you've reviewed
2018-04-16 15:37:40   can't remember anything else, it's too early
2018-04-16 15:37:42   next
2018-04-16 15:38:30think it's me.
2018-04-16 15:38:32  right
2018-04-16 15:38:36* Fixed output identifier position in 
D12151
2018-04-16 15:38:36   Diff 12151 "Position output identifiers 
with respect to scaling factors" [Closed] https://phabricator.kde.org/D12151
2018-04-16 15:38:38* Still working on T4465. My first 
approach didn't work out well and now I try to reduce complexity of my first 
approach. Maybe I will have a working prototype today. But I could need some 
help on Qt model programming.
2018-04-16 15:38:38   

D8626: DDCUtil: Improved DDCUtil support for brightness control

[Kai Uwe Broulik]

broulik added a comment.


  lgtm

INLINE COMMENTS

> CMakeLists.txt:58
> +  find_package(DDCUtil REQUIRED)
> +  add_definitions(-DWITH_DDCUTIL)
>set_package_properties(DDCUtil

we typically use `HAVE_xxx` but fine with me

> ddcutilbrightness.h:27
>  
> -#ifdef WITH_DDCUTIL
> +#if WITH_DDCUTIL
>  #include 

Doesn't this need to stay `ifdef`?

REPOSITORY
  R122 Powerdevil

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

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

D11801: remove m_desktopContainments

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:5898499c1f8f: remove m_desktopContainments (authored by 
mart).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11801?vs=31201=32264

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

AFFECTED FILES
  shell/shellcorona.cpp
  shell/shellcorona.h

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

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

[David Edmundson]

davidedmundson added a comment.


  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.

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

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

[David Edmundson]

davidedmundson added a comment.


  cool, seems mostly fine. One minor style comment

INLINE COMMENTS

> wallpaperplugin.cpp:26
> +m_newStuffDialog = new KNS3::DownloadDialog( 
> QString::fromLatin1("wallpaperplugin.knsrc") );
> +KNS3::DownloadDialog *strong = m_newStuffDialog.data();
> +strong->setTitle(i18n("Download Wallpaper Plugins"));

You don't need to do this.

QPointer has a custom -> operator

So m_newStuffDialog->setTitle works just fine.

Same for in newStuffFinished

REPOSITORY
  R131 Plasma Wallpapers

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

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