[Kde-pim] Review Request: Remove close button on the Article tab in Akregator

[Laurent Montel]

> On July 10, 2012, 8:13 p.m., Allen Winter wrote:
> > I tested this patch and I don't like the new behavior.
> > 
> > For me, the first close button on the first tab disappears immediately after I open another tab, for example with I click on the "The complete story" link.
> > 
> > As long as more than 1 tab is open, I think the close button should be available on all tabs.
> 
> Phương Lê Hoàng wrote:
>     You can read the bug 173067. The Article tab is the first tab and it can not be repositioned to second or so on. In the bug 173067, ostefield has disabled the close function for the Article tab, but he didn't remove the close button altogether with its function. What I want to do is to completely remove that close button, so users won't get confused about whether that tab can be closed or not.
> 
> Laurent Montel wrote:
>     If button is disable is enough.
>     And if Allen tested it and see "For me, the first close button on the first tab disappears immediately after I open another tab" it's a new bug.
>     So for me this patch is not good (Ok not tested yet, but by default disable button is enough)
> 
> Phương Lê Hoàng wrote:
>     What Allen said is the wanted behaviour. The Article tab (aka the first tab) is the default tab and it can not be closed. Keeping the close button there is unnecessary and causes confusing for users. This duplicated bug 171360 explains what the users want. Disable button is not enough because it still can be clicked, though after clicking, nothing happens.
> 
> Phương Lê Hoàng wrote:
>     If you don't have time to test, you can take a look in these screen shot:
>     New: http://imageshack.us/photo/my-images/23/newakregator.png/
>     Old: http://imageshack.us/photo/my-images/402/oldakregator.png/
>     (The close buttons are different because of different themes)
>     You can see in the old one, the button is still active. But clicking on it does nothing because of the fix for bug 173067. What I want to do is make it a proper fix, as in the new one.
> 
> Laurent Montel wrote:
>     Ok I tested it.
>     It's right your patch is good but not when we start akregator.
>     We see close button in first page.
>     So we must to fix it first and after we can push it.

diff --git a/akregator/src/tabwidget.cpp b/akregator/src/tabwidget.cpp
index 82aa5bb..faa83b6 100644
--- a/akregator/src/tabwidget.cpp
+++ b/akregator/src/tabwidget.cpp
@@ -89,6 +89,8 @@ public:
 void TabWidget::Private::updateTabBarVisibility()
 {
     q->setTabBarHidden( ( q->count() <= 1 ) && !Settings::alwaysShowTabBar() );
+    if (q->count() >= 1 && Settings::closeButtonOnTabs())
+        q->tabBar()->tabButton(0, QTabBar::RightSide)->hide();
 }
 
 TabWidget::TabWidget(QWidget * parent)


"q->count() >= 1" it fixes this bug and it's ok for me


- Laurent


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


On July 10, 2012, 1:58 p.m., Phương Lê Hoàng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105504/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 1:58 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> Completely remove the close button on the Article tab in Akregator.
> Related bug: https://bugs.kde.org/show_bug.cgi?id=173067 (This bug is marked as solved).
> 
> 
> Diffs
> -----
> 
>   akregator/src/tabwidget.cpp 82aa5bb 
> 
> Diff: http://git.reviewboard.kde.org/r/105504/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Phương Lê Hoàng
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/