This is for the benefit of users who find changing the window height
through the configuration window overly tedious.
This may cause slight flickering during resizing.
FEATURE: 154686
FIXED-IN: 20.04.0
hein | |
andrewfhou | |
ngraham |
Yakuake |
This is for the benefit of users who find changing the window height
through the configuration window overly tedious.
This may cause slight flickering during resizing.
FEATURE: 154686
FIXED-IN: 20.04.0
Run patched Yakuake.
Drag Titlebar up and down the screen.
Yakuake should automatically update its height configuration while being dragged.
Lint Skipped |
Unit Tests Skipped |
app/titlebar.cpp | ||
---|---|---|
143 | KDE coding style: space between if and ( and also between the ) and { | |
144 | No need to use this; just do parent() directly (and in fact, this means you don't even need to declare it as a variable, just use it directly in the below lines. | |
149 | space between if and ( | |
150 | KDE coding style: use braces even for single-line statements following conditionals | |
151 | wrong indentation. Also, this could be an else if | |
152 | use braces | |
app/titlebar.h | ||
59 | Indentation doesn't match the above lines |
Adjusted formatting to fit KDE style guidelines.
Kept the MainWindow* parent declaration as it is needed for compilation.
Did not change to if/else as no subsequent else statement follows.
The resize should happen in the same steps as the config allows (and reads/writes), otherwise the UI will end up being confused, in particular the menu.
app/titlebar.cpp | ||
---|---|---|
144 | Coding style: Please don't use a C-style cast, use a C++ one. |
I re-read the MainWindow::setWindowHeight code, it updates the configuration so UI confusion will not occur.
Resizing the window works great, and the 'flickering' effect is relatively minor. However, the cursor should change when it is over the titlebar to indicate to the user that they are able to drag to resize it - ideally the vertical double arrow cursor icon
@ryanmccoskrie sorry this got lost. Would you be interested in finishing it up so we can get it landed?
Dragging and icon seem to be working for me! I think it should be good once you take care of the style change requests
LGTM. @mweepigeon? Since you requested changed, you'll nedd to re-review and change your status to Accepted if you're happy with it now.
@ryanmccoskrie can you provide an email address so we can land this patch with correct attribution? Thanks again for the lovely contribution!
On a related note, I uploaded another patch that reformats the rest of Yakuake's if statements to match the ones here.
app/titlebar.cpp | ||
---|---|---|
3 | You've been working on Yakuake since 2008? :) |
app/titlebar.cpp | ||
---|---|---|
157 | Those values (100 and 10) seem a bit arbitrary to me. Is there any reasoning behind them? |
app/titlebar.cpp | ||
---|---|---|
157 | Sorry, ignore my comment. Didn't realize newHeight is a percentage here. Could potentially rename the variable to make that clearer. |