[rekonq] Review Request: Refactoring of some part of the tab bar

Andrea Diamantini adjam7 at gmail.com
Tue Aug 14 16:13:04 UTC 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105938/#review17400
-----------------------------------------------------------


Closing as submitted. If you'd like to go again for your last commit, please open another request. Thanks.

- Andrea Diamantini


On Aug. 8, 2012, 9:43 p.m., Yoann Laissus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105938/
> -----------------------------------------------------------
> 
> (Updated Aug. 8, 2012, 9:43 p.m.)
> 
> 
> Review request for rekonq.
> 
> 
> Description
> -------
> 
> The branch is here : http://github.com/Arakmar/rekonq/tree/tabBarRefactoring
> 
> The original goal of this merge request was to (finally) fix all problems we have with the interaction between the tabbar and KMessageWidget.
> While investigating for this problem, I also cleaned and fixed some bugs with the add tab button and tab previews which was hidden by the previous implementation
> That's why I decided to split that in independant parts. Only the last one depends on the first two. Those two parts can be safely backported in the 1.0 branch as it's only fixes.
> 
> First part : add tab button refactoring (6f291145ba8a4)
> This commit clean the updateTabBar method by splitting it in two methods (updateAddTabButton and updateTabBarVisibility) as it's two things very different.
> The position of the new tab button is now updated only on layout change of the tab bar.
> This method has the benefit to not call the slot in every method which add a tab.
> It also fixes bug 304325 and some nasty bugs with KMessageWidget due to non updated position.
> 
> Second part : Tab preview refactoring (b83d6a, 7e3158, f5f69)
> This part is mainly here to prepare the last one.
> Those commits introduce a fixed size (200px) for tab previews to fix issues with too big preview on very high ratio. (eg : preview of tabs for a 200x900 window)
> 200 can be discussed but it's the default size of a tab. (The size hint of the mainview is always the same, even on a resolution change ...)
> 
> Third part : Tab width refactoring (2cf613)
> The problem is described here :
> https://git.reviewboard.kde.org/r/102651/
> 
> When a KTabWidget appears, the tab width grows up. And without the first part of this MR, the add tab button isn't even updated.
> 
> The previous fix has been removed by some refactoring of Application and MainWindow.
> But I think we can't rely on this solution anymore, it's not the proper way to fix that.
> 
> That's why I have replaced the existing solution by a more configurable one.
> There is now the choice between two options (in the Tab section) :
> 
> - Fixed configurable size (Default to 200 and minimum 100)
> It's the replacement of the previous behaviour.
> We can't rely on MainView::tabSizeHint() as it's the same value no matter the resolution of the screen.
> Moreover, all major browsers use a fixed (configurable) size (Firefox - 250 px, Chrome - 200 px)
> So I think it's a lot safer to use that.
> 
> - Automatic size depending on the title size:
> It's the default behaviour of KTabBar (Konqueror)
> 
> (Sorry for this big description :D )
> 
> 
> This addresses bug 304325.
>     /show_bug.cgi?id=304325
> 
> 
> Diffs
> -----
> 
>   src/application.cpp 2127ca9 
>   src/mainview.h 9378f4b 
>   src/mainview.cpp 861a6b1 
>   src/rekonq.kcfg 1fb3c1c 
>   src/settings/settings_tabs.ui 4e19545 
>   src/settings/tabswidget.h 056f030 
>   src/settings/tabswidget.cpp 67829f8 
>   src/tabbar.h 161612c 
>   src/tabbar.cpp ac257d1 
>   src/tabpreviewpopup.h 5c62102 
>   src/tabpreviewpopup.cpp 63ff542 
> 
> Diff: http://git.reviewboard.kde.org/r/105938/diff/
> 
> 
> Testing
> -------
> 
> Tested on multiple computers with different resolutions.
> 
> 
> Screenshots
> -----------
> 
> Automatic width
>   http://git.reviewboard.kde.org/r/105938/s/665/
> Fixed width
>   http://git.reviewboard.kde.org/r/105938/s/666/
> Tab settings
>   http://git.reviewboard.kde.org/r/105938/s/667/
> 
> 
> Thanks,
> 
> Yoann Laissus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/rekonq/attachments/20120814/0df4d3e2/attachment-0001.html>


More information about the rekonq mailing list