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

On October 25th, 2012, 9:44 a.m., Aaron J. Seigo wrote:

there are a number of style issues with this patch, but the real problem is setting it to a hardcoded 1/3rd of the panel.

i would suggest that the real underlying problem here is that the m_tabBar sizehint keeps changing so radically.
I have limited internet access from my development machine during these days, but I'll try addressing the m_tabBar sizeHint issue ASAP. Yet, even if that bug is fixed, for 8+ activities, I personallly prefer the tab bar to take only a given amount of space (independent of the number of tabs) and display arrows, see

http://mail.kde.org/pipermail/plasma-devel/2011-November/017890.html

I will work through the style issues if possible, but there are some comments below I'd like to discuss.

On October 25th, 2012, 9:44 a.m., Aaron J. Seigo wrote:

plasma/generic/applets/activitybar/activitybar.cpp (Diff revision 1)
37
void setPreferredSize(ActivityBar & bar) {
this should be a member of the class (the hint should have been passing in *this constantly); perhaps call it 'calculatePreferredSize" to avoid conflicting with setPreferredSize
I obviously chose this design to avoid braking the ABI.

On October 25th, 2012, 9:44 a.m., Aaron J. Seigo wrote:

plasma/generic/applets/activitybar/activitybar.cpp (Diff revision 1)
42
        bar.setPreferredSize(QSizeF(containmentRect.width() / 3, containmentRect.height()));
43
        break;
44
    case Plasma::Vertical:
45
        bar.setPreferredSize(QSizeF(containmentRect.width(), containmentRect.height() / 3));
1/3rd looks wrong and is going to give incorrect results in several situations (e.g. when pref size of all plasmoids fit, but the activity bar has fewer buttons than the space it is alloted)
That's intended. If the space alloted is greater than the size required to display the few tabs, the tabs expand to fit the size and the look is similar to what we have now (which is, by the way, horrible) just not constantly expanding. On the other hand, if you have 8+ tabs, they will not fit in the 1/3 size and the tab bar will display arrows for scrolling left or right.

Taking any approach that varies with the amount of tabs is actually against the principle of least astonishment, from my point of view.

- David


On October 22nd, 2012, 2:27 a.m., David Narváez wrote:

Review request for Plasma.
By David Narváez.

Updated Oct. 22, 2012, 2:27 a.m.

Description

The main issue was caused by the fact that asking the tab bar native widget to provide a size hint was giving a different size every time which caused the activity bar to expand or shrink in the presence of another expanding widget (in my case, the taskbar), so this patch decides a size hint from the size of the containment (one third of the width of height depending on the form factor) but the size policy is set to expand when the activity bar is the only widget in the containment.

Testing

1) Add a panel with an actyivity bar and a task bar
2) Add a panel with an activty bar only
3) Change activities

Before this patch, the activity bar in the panel with a task bar shrinks or enlarges with every activity change; while the size of the activity bar in the second panel remains constant. After this patch, the size of the activity bar in both panels remains constant.
Bugs: 225078

Diffs

  • plasma/generic/applets/activitybar/activitybar.cpp (e66bf04)

View Diff