Review Request 119115: DolphinTabWidget

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119115/#review62752
-----------------------------------------------------------


Thanks for the update! Sorry for the late response - I was a bit busy at work last week.

Looks good already (and much better than the last version, thanks for fixing the issues!). I still have a few questions though.


dolphin/src/dolphinmainwindow.cpp
<https://git.reviewboard.kde.org/r/119115/#comment43495>

    Maybe the active view could be a parameter of the signal, i.e.,
    
    activeViewChanged(DolphinViewContainer* newViewContainer)?
    
    This access would not be needed then. And maybe the activeViewContainer() function in DolphinTabWidget could even be removed?



dolphin/src/dolphinmainwindow.cpp
<https://git.reviewboard.kde.org/r/119115/#comment43496>

    The old code simply kept the connections to the signals of all views and URL navigators at all times.
    
    Could you comment on why you propose to introduce the disconnections and connections now?



dolphin/src/dolphintabwidget.cpp
<https://git.reviewboard.kde.org/r/119115/#comment43497>

    As far as I can see, this signal does not exist at all?



dolphin/src/dolphintabwidget.cpp
<https://git.reviewboard.kde.org/r/119115/#comment43498>

    It seems that this signal does not exist either.


- Frank Reininghaus


On July 15, 2014, 9:26 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119115/
> -----------------------------------------------------------
> 
> (Updated July 15, 2014, 9:26 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Implemented DolphinTabWidget class to encapsulate the tab handling from DolphinMainWindow.
> 
> Removed the method for text squeezing because the QTabBar can do this automatically.
> 
> The tab sizes are different, any idea how to fix this?
> 
> 
> Diffs
> -----
> 
>   dolphin/src/CMakeLists.txt 7b0210a 
>   dolphin/src/dolphinmainwindow.h 9c7f185 
>   dolphin/src/dolphinmainwindow.cpp 41bd626 
>   dolphin/src/dolphintabwidget.h PRE-CREATION 
>   dolphin/src/dolphintabwidget.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119115/diff/
> 
> 
> Testing
> -------
> 
> Works great so far.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140720/08d7a653/attachment.htm>