| rkflx |
| Gwenview |
The URL navigator was slightly narrower than full width until you
went to fullscreen and back, at which point it fixed itself. This
was because the fullscreen toolbars were visible on app launch,
but hidden when exiting fullscreen.
This patch simply hides them at launch.
Left before:
Left after:
Right before:
Right after:
Open Gwenview. The left and right of the URL Navigator in Browse
should be flush with the edge (see screenshots).
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
@rkflx This patch will have a git conflict with D10880: Improve background color code for URL Navigator and adjacent fullscreen toolbars. Will this cause issues, given it's also in the stable branch and not master?
If this patch landed first, I could rebase the other patch, correct? But if it's the other way round, the conflict would arise when merging stable with master, and therefore whoever lands it would have to sort out the conflict.
Is there a best practice with this sort of situation?
Nice ;) I'm curious: How did you come up with the fix?
Patch LGTM, no revision needed. 💯 🎆
(see screenshots)
As a tip for later screenshots: Use the default colour scheme, where the issue would've been a bit easier to see. Also, Spectacle has a setting to remember the rectangular region (should we make it the default!?), giving you more consistent before/after screenshots.
Tried to provoke a merge conflict, but regardless of the order I apply both patches to master, there is none. Do you mean that the comment you added will be duplicated? If so, thanks for the heads-up.
I think Git is right in adding both changes, because it cannot know both are duplicates. The situation would be different if you duplicated a whole commit, e.g. via cherry-picking. In general you should try to only add the comment in a single place, and watch out it ends up on top of the whole code block in the end.
While there is a concept of dependent revisions, it does not apply if you work on different branches. What to do now? I'll land this patch and (as usual) merge stable to master. Then you can rebase your other patch on top of that.
I used Qt Designer and GammaRay to look at the layout and noticed the two fullscreen toolbars were initialised to the left and right with widths of 0, but still apparently took up room.
Since going to fullscreen fixed it, I looked at setFullScreenMode and saw it was setting the visibility on these toolbars. It natually followed that if they are hidden when exiting fullscreen, they should be hidden at the start.
As a tip for later screenshots: Use the default colour scheme, where the issue would've been a bit easier to see.
Will do!
Also, Spectacle has a setting to remember the rectangular region (should we make it the default!?), giving you more consistent before/after screenshots.
I actually looked for this! Of course, I totally missed Preferences... in the save menu. Is a bit of an odd place to put it (boy this KDE dev is a rabbit hole...)
Tried to provoke a merge conflict, but regardless of the order I apply both patches to master, there is none. Do you mean that the comment you added will be duplicated? If so, thanks for the heads-up.
Yep, I wasn't sure exactly what git would do, just that I was modifying the same lines of code, and as you say, duplicating the comment.
I think Git is right in adding both changes, because it cannot know both are duplicates. The situation would be different if you duplicated a whole commit, e.g. via cherry-picking. In general you should try to only add the comment in a single place, and watch out it ends up on top of the whole code block in the end.
Okay if a similar thing happens again, I'll only add the comment in one patch.
Cool, glad it helped.
Also, Spectacle has a setting to remember the rectangular region (should we make it the default!?), giving you more consistent before/after screenshots.
I actually looked for this! Of course, I totally missed Preferences... in the save menu. Is a bit of an odd place to put it (boy this KDE dev is a rabbit hole...)
We changed it! See T7841 and 0e04f7debc78 for the result.
Now I only want to convince Nate that by default we should enable Remember Rectangular Region ;) I'd be interested to know what you think about it.
Awesome! The new design looks much better at first glance.
Now I only want to convince Nate that by default we should enable Remember Rectangular Region ;) I'd be interested to know what you think about it.
Given that it doesn't impede workflow if you want to define a new region, I don't see any major issue with it. It could also help discoverability.
I'm not sure though. For users that never have any use for consistent region across screenshots, having the previous region always there when taking a new region might feel awkward.