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

2018-05-09 Thread Scott Harvey
This revision was automatically updated to reflect the committed changes. Closed by commit R124:19712a1d22b2: Enlarge default window size. Use (adjusted) calculation in `SizeHint` to… (authored by sharvey). REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE

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

2018-05-04 Thread Nathaniel Graham
ngraham added a comment. In D12252#254928 , @zzag wrote: > I've just noticed that it's calling `setMinimumSize`. Why? (especially, with such a big minimum size) > > What about icon based view mode, i.e. the old settings layout? Hmm,

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

2018-05-04 Thread Scott Harvey
sharvey added a comment. In D12252#254928 , @zzag wrote: > I've just noticed that it's calling `setMinimumSize`. Why? (especially, with such a big minimum size) > > What about icon based view mode, i.e. the old settings layout? The new

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

2018-05-04 Thread Scott Harvey
sharvey updated this revision to Diff 33667. sharvey marked an inline comment as done. sharvey added a comment. - Remove redundant size setting REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12252?vs=33208=33667 BRANCH arcpatch-D12252 REVISION

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

2018-04-27 Thread Vlad Zagorodniy
zzag added a comment. I've just noticed that it's calling `setMinimumSize`. Why? (especially, with such a big minimum size) What about icon based view mode, i.e. the old settings layout? The new settings layout(with sidebar) is still usable with window sizes smaller than 1024x700.

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

2018-04-27 Thread Nathaniel Graham
ngraham added inline comments. INLINE COMMENTS > SettingsBase.cpp:107 > > +setMinimumSize(SettingsBase::sizeHint()); > + You've already added a `setMinimumSize()` call below (under the "// enforce minimum window size" comment); do we need this second one here? REPOSITORY R124 System

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

2018-04-27 Thread Scott Harvey
sharvey marked an inline comment as done. sharvey added inline comments. INLINE COMMENTS > ngraham wrote in SettingsBase.cpp:108 > Do we actually need this? It wasn't referenced anywhere else in the project. Probably a leftover from times long ago. REPOSITORY R124 System Settings REVISION

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

2018-04-27 Thread Scott Harvey
sharvey updated this revision to Diff 33208. sharvey added a comment. - Remove old, unused call to KConfig "MainWindow" REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12252?vs=32814=33208 BRANCH enlarge-default-size (branched from master)

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

2018-04-26 Thread Nathaniel Graham
ngraham added inline comments. INLINE COMMENTS > SettingsBase.cpp:108 > + > +KConfigGroup mainWindowCfg = > KSharedConfig::openConfig()->group("MainWindow"); > + Do we actually need this? REPOSITORY R124 System Settings REVISION DETAIL https://phabricator.kde.org/D12252 To: sharvey,

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

2018-04-25 Thread Nathaniel Graham
ngraham added a comment. Ping! @davidedmundson or anyone else, any remaining objections to this? REPOSITORY R124 System Settings REVISION DETAIL https://phabricator.kde.org/D12252 To: sharvey, ngraham, mart, davidedmundson Cc: davidedmundson, cfeck, zzag, plasma-devel, ragreen, Pitel,

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

2018-04-22 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. @davidedmundson? REPOSITORY R124 System Settings REVISION DETAIL https://phabricator.kde.org/D12252 To: sharvey, ngraham, mart, davidedmundson Cc: davidedmundson, cfeck, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai,

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

2018-04-22 Thread Scott Harvey
sharvey updated this revision to Diff 32814. sharvey added a comment. - Remove scale factor entirely; set minimum size REPOSITORY R124 System Settings CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12252?vs=32368=32814 BRANCH enlarge-default-size (branched from master)

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

2018-04-22 Thread Nathaniel Graham
ngraham added inline comments. INLINE COMMENTS > sharvey wrote in SettingsBase.cpp:82 > So should I just run the "preferred size" (1024x760) through the `boundedTo` > function for odd-sized screens? In other words, ditch the scale factor > completely? As in: > > const QSize screenSize = >

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

2018-04-17 Thread Scott Harvey
sharvey added inline comments. INLINE COMMENTS > davidedmundson wrote in SettingsBase.cpp:82 > > qreal factor = qBound(1.0, > > QGuiApplication::primaryScreen()->devicePixelRatio()/96., 3.0); > > I know you were told to change to this, but don't do that. > > Qt will scale the size you give it

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

2018-04-17 Thread Scott Harvey
sharvey added a comment. In D12252#248478 , @cfeck wrote: > OMG, for whatever reason, I was assuming this is about the file dialog window size. You are working on too many things at once, Nathan :) And don't forget about poor, poor,

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

2018-04-17 Thread Nathaniel Graham
ngraham added a comment. In D12252#248478 , @cfeck wrote: > OMG, for whatever reason, I was assuming this is about the file dialog window size. You are working on too many things at once, Nathan :) Hehe, if you want to review some of

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

2018-04-17 Thread Christoph Feck
cfeck added a comment. OMG, for whatever reason, I was assuming this is about the file dialog window size. You are working on too many things at once, Nathan :) REPOSITORY R124 System Settings REVISION DETAIL https://phabricator.kde.org/D12252 To: sharvey, ngraham, mart, davidedmundson

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

2018-04-17 Thread David Edmundson
davidedmundson requested changes to this revision. davidedmundson added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > zzag wrote in SettingsBase.cpp:82 > You forgot `const`. > > Also, you could multiply whole QSize by `factor` so Qt will round width and >

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

2018-04-17 Thread Nathaniel Graham
ngraham added a comment. In D12252#248458 , @cfeck wrote: > I still object to enforce a minimum size. On my main system, I use a 4K screen, and having a file dialog span nearly the complete screen is irritating, and mostly unusable because I

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

2018-04-17 Thread Christoph Feck
cfeck added a comment. I still object to enforce a minimum size. On my main system, I use a 4K screen, and having a file dialog span nearly the complete screen is irritating, and mostly unusable because I have to travel a lot to reach buttons. What is wrong with offering a good default

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

2018-04-17 Thread Scott Harvey
sharvey updated this revision to Diff 32368. sharvey added a comment. - Add checking & sizing calc for smaller or rotated screens `availableSize()` returns size minus any size reserved by the window manager for taskbars, etc. Multiply by 0.9 to prevent systemsettings from going "full

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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)

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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.

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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.

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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)

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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

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

2018-04-16 Thread 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 -

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

2018-04-16 Thread 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