--===============1401032002021994308== Content-Type: multipart/alternative; boundary="===============7300622475896257663==" --===============7300622475896257663== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Oct. 25, 2012, 9:44 a.m., Aaron J. Seigo wrote: > > there are a number of style issues with this patch, but the real proble= m is setting it to a hardcoded 1/3rd of the panel. > > = > > i would suggest that the real underlying problem here is that the m_tab= Bar sizehint keeps changing so radically. I have limited internet access from my development machine during these day= s, but I'll try addressing the m_tabBar sizeHint issue ASAP. Yet, even if t= hat bug is fixed, for 8+ activities, I personallly prefer the tab bar to ta= ke only a given amount of space (independent of the number of tabs) and dis= play 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 commen= ts below I'd like to discuss. > On Oct. 25, 2012, 9:44 a.m., Aaron J. Seigo wrote: > > plasma/generic/applets/activitybar/activitybar.cpp, line 37 > > > > > > this should be a member of the class (the hint should have been pas= sing in *this constantly); perhaps call it 'calculatePreferredSize" to avoi= d conflicting with setPreferredSize I obviously chose this design to avoid braking the ABI. > On Oct. 25, 2012, 9:44 a.m., Aaron J. Seigo wrote: > > plasma/generic/applets/activitybar/activitybar.cpp, lines 42-45 > > > > > > 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 simil= ar 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 th= e 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 ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106978/#review20847 ----------------------------------------------------------- On Oct. 22, 2012, 2:27 a.m., David Narv=C3=A1ez wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106978/ > ----------------------------------------------------------- > = > (Updated Oct. 22, 2012, 2:27 a.m.) > = > = > Review request for Plasma. > = > = > Description > ------- > = > The main issue was caused by the fact that asking the tab bar native widg= et to provide a size hint was giving a different size every time which caus= ed the activity bar to expand or shrink in the presence of another expandin= g 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. > = > = > This addresses bug 225078. > http://bugs.kde.org/show_bug.cgi?id=3D225078 > = > = > Diffs > ----- > = > plasma/generic/applets/activitybar/activitybar.cpp e66bf04 = > = > Diff: http://git.reviewboard.kde.org/r/106978/diff/ > = > = > 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 act= ivity bar in both panels remains constant. > = > = > Thanks, > = > David Narv=C3=A1ez > = > --===============7300622475896257663== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.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 limi=
ted internet access from my development machine during these days, but I=
9;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 ar=
rows, 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 commen=
ts 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(Activ=
ityBar & bar) {

  


 




  
this shou=
ld be a member of the class (the hint should have been passing in *this con=
stantly); perhaps call it 'calculatePreferredSize" to avoid confli=
cting 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.w=
idth() / 3, containmentR=
ect.height()));

  


  

    
    

    43
    
        break;

  


  

    
    

    44
    
    case Plasma::Vertical:=


  


  

    
    

    45
    
        bar.setPreferredSize=
(QSizeF(containmentRect.w=
idth(), containmentRect.height() / 3));

  


 




  
1/3rd loo=
ks 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 button=
s than the space it is alloted)

 









That's intended. If the space alloted is greater than the size r=
equired to display the few tabs, the tabs expand to fit the size and the lo=
ok 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 no=
t fit in the 1/3 size and the tab bar will display arrows for scrolling lef=
t 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=C3=A1ez wrote:









 

  


Review request for Plasma.


By David Narv=C3=A1ez.




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








Descripti=
on 

 

 

  
   
The main issue was caused by the fact that asking the tab ba=
r native widget to provide a size hint was giving a different size every ti=
me which caused the activity bar to expand or shrink in the presence of ano=
ther expanding widget (in my case, the taskbar), so this patch decides a si=
ze 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 <=
/h1>


 

  
   
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 activ=
ity bar in both panels remains constant.

  		
 








 Bugs: 


 225078







Diffs=
 


    

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





View Diff





  		
 











  

 



--===============7300622475896257663==--

--===============1401032002021994308==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

--===============1401032002021994308==--