Recent

Author Topic: Aesthetic bugfix of Lazarus's Component Palette  (Read 42244 times)

zeljko

  • Hero Member
  • *****
  • Posts: 1594
    • http://wiki.lazarus.freepascal.org/User:Zeljan
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #45 on: April 29, 2014, 08:41:34 am »
What's next ?

Fix regression on gtk2 (freeze) :)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9792
  • Debugger - SynEdit - and more
    • wiki
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #46 on: April 29, 2014, 01:56:32 pm »
I will look at your patch later. Bit busy right now.

Yep, there is a bug somewhere in LCL-GTK. And it is triggered by the layout change.

So now the IDE hangs on gtk2

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9792
  • Debugger - SynEdit - and more
    • wiki
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #47 on: April 29, 2014, 02:38:43 pm »
Quote
Fix regression on gtk2 (freeze)

And this is why I am so extremely picky, about what I think is correct and what not.

Well in this case the patch was correct. (Never mind the change I did, which I could only do, because the patch was good before).

But it still triggered a bug. The bug is somewhere else (not yet found). But that doesn't matter. I did commit the code that triggered it, and made the IDE freeze.
So I was now forced to look into it.

In the end, I committed a workaround, because the actual bug is in code that I don't know well, and I can only leave it for the attention of someone who does know that code.


The point is, If the code in the patch had contained anything that I would have been doubtful of before the commit, then I would have really been upset about myself. (Because then I would have had to spent time tracking a bug, because I committed something, that I already had thought of as not being good enough).

With the code having been judged as ok by me, the story is different.. I still had to look at it, but well that happens (it happens with my own changes too). At least I took no unnecessary risk.
And at least I could quickly tell the bug was not likely in this code, but someplace else. That is, I could tell that, using "ClientSizeWithoutBar" is meant to work, and meant to work anywhere and anytime, and in any event too
« Last Edit: April 29, 2014, 02:41:49 pm by Martin_fr »

taazz

  • Hero Member
  • *****
  • Posts: 5368
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #48 on: April 29, 2014, 02:53:57 pm »
@taazz
Slightly OT, but I'm intrigued as to why you choose an OS that annoys you and hinders code development [snip...]
I'm genuinely interested in (possible) advantages of Win8 over its predecessors and non-Windows alternatives.

The most notable plus I can say about windows 8 is the language support that the system has. For example on the laptop I currently have there are 2 user accounts the owner's and mine the owner's is translated in his mother language everything every single aspect of the OS as far as I have checked is translated, while my account is in English, the default installation language and all it took from me was to connect it to the net download and install a language pack.

That one is a big plus from me.
Good judgement is the result of experience … Experience is the result of bad judgement.

OS : Windows 7 64 bit
Laz: Lazarus 1.4.4 FPC 2.6.4 i386-win32-win32/win64

x2nie

  • Hero Member
  • *****
  • Posts: 515
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #49 on: April 29, 2014, 07:18:01 pm »
What's next ?

Fix regression on gtk2 (freeze) :)
I've looked at the trunk. I know what is going on.


Martin, can we clean up the code from {IFDEF GTK2} now ?
I found another solution that simulate requirement of ClientSizeWithoutBar & ClientSizeWitBar
This will be done by calculate the Scrollbox.Width directly :





Code: [Select]
    (* ==== PLEASE REMOVE IT =========
    {$IFDEF LCLGTK2}
    MaxBtnPerRow:=((ScrollBox.ClientWidth - 30 - ButtonX) div ComponentPaletteBtnWidth);
    {$ELSE}
    MaxBtnPerRow:=((ScrollBox.VertScrollBar.ClientSizeWithoutBar - ButtonX) div ComponentPaletteBtnWidth);
    // If we need to wrap, make sure we have space for the scrollbar
    if MaxBtnPerRow < ButtonTree.Count then
      MaxBtnPerRow:=((ScrollBox.VertScrollBar.ClientSizeWithBar - ButtonX) div ComponentPaletteBtnWidth);
    {$ENDIF}
    ======= END REMOVE IT ========== *)
   
    MaxBtnPerRow:=((ScrollBox.Width - ButtonX) div ComponentPaletteBtnWidth);   //without scrollbar
    if MaxBtnPerRow<1 then MaxBtnPerRow:=1;
    // Reduce visible button if there is multiline (scrollbar visible)
    Rows := ButtonTree.Count div MaxBtnPerRow;
    if MaxBtnPerRow * Rows < ButtonTree.Count then
       Inc(Rows);
    if Rows > 1 then       //multi line?
       Dec(MaxBtnPerRow);  //reduced.


What do you think ?
When you were logged in, you can see attachments.
Lazarus Github @ UbuntuCinnamon-v22.04.1 + LinuxMintDebianEdition5

x2nie

  • Hero Member
  • *****
  • Posts: 515
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #50 on: April 29, 2014, 07:32:02 pm »
I can't give patch for now.
since I am also in the middle modifying the unit (vertical-centered button) which is not yet finish.


Update: comment/explanation added


MaxBtnPerRow:=((ScrollBox.Width - ButtonX) div ComponentPaletteBtnWidth);   // without scrollbar, first.
if MaxBtnPerRow<1 then MaxBtnPerRow:=1;                                     // avoid division by zero

// Now, reduce visible button if there is multiline (scrollbar visible)

Rows := ButtonTree.Count div MaxBtnPerRow;               // its maybe wrong.
if MaxBtnPerRow * Rows < ButtonTree.Count then           // correction of integer truncate...
   Inc(Rows);                                             // since we don't use Math.CEIL(a/b);
if Rows > 1 then          //multi line?
   Dec(MaxBtnPerRow);      //reduced.
« Last Edit: April 29, 2014, 07:35:30 pm by x2nie »
When you were logged in, you can see attachments.
Lazarus Github @ UbuntuCinnamon-v22.04.1 + LinuxMintDebianEdition5

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9792
  • Debugger - SynEdit - and more
    • wiki
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #51 on: April 29, 2014, 07:43:37 pm »
Quote
Code: [Select]
    if Rows > 1 then       //multi line?
       Dec(MaxBtnPerRow);  //reduced.

Not always correct.

1) In theory on some widget, scrollbar can take more space than one item.

2) if item is 20 pixel width, and scroll bar 10, and total 55, then you need not decrease it at all.

-----------------
The bug in gtk needs to be fixed.
Using ClientSizeWithoutBar  is the correct calculation.

So that will be kept.

x2nie

  • Hero Member
  • *****
  • Posts: 515
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #52 on: April 29, 2014, 08:20:07 pm »
Well, I hope the bug got fixed in few days.


Anyway, I've finished my centered-vertically ComponentPalette buttons.
You can disagree with some part, at least I am satisfied with the (aesthetic) result.


Below is the patch
 8-)

When you were logged in, you can see attachments.
Lazarus Github @ UbuntuCinnamon-v22.04.1 + LinuxMintDebianEdition5

x2nie

  • Hero Member
  • *****
  • Posts: 515
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #53 on: April 29, 2014, 08:40:39 pm »
Guys,
my prior patch is also prepared to handle layout in multiple rows visibility of the component buttons.


Now, how to change the maximum height of IDE form? which unit.pas? which procedure?
I am very curious whether it works as expected.
When you were logged in, you can see attachments.
Lazarus Github @ UbuntuCinnamon-v22.04.1 + LinuxMintDebianEdition5

x2nie

  • Hero Member
  • *****
  • Posts: 515
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #54 on: April 30, 2014, 08:53:47 am »
After few hours searching in IDE codes, I found the way to increase IDE mainform's Constraint.MaxHeight.
It is required for showing buttons in Component Palette shown in multiple row.
And it works.  :D 
I mean my aesthetic expectation/calculation is also perfect in multi-row mode.


My aesthetic philosophy of how component button's position is simple:
* Keep the Arrow/SelectButton in home, it should never go anywhere when scrollbox is scrolled.
   
* All buttons in component palette should be center-aligned-vertically when in single row.
   It is default behavior, since current Lazarus allow only single row.
   it is the final-touch of aesthetic aspect: vertical margin distributed equally (margin top = margin bottom).
   So, While Lazarus should never show component button in multiple row by design,
   The button will always shown centered vertically (not top aligned as in current release)


But, In case Lazarus could show component buttons in multirow (as my experiment), the additional rules is follow:
* All buttons should reflect to Top align in multi-row mode.
  This decision is better (in aesthetic perspective) instead of vertically-center.
   > when there is too large space available. Too large := ScrollBox.Height >= ButtonHeight * 2
   > When there is too small space available. Too small := ScrollBox.Height < ButtonHeight;


Otherwise (if single row or assumed as single row), vertical-center alignment shall be performed.


--------
But, you know we are in early stage of such improvement.
I cant guarantee that my solution is free from bug.
For instance, I found aesthetic bug from my prior patch:
The arrow-button is sometime not in it home when IDE form is resized. It shall always there anytime.
But, it is easy to see the arrow-button again:
click the Vertical-scrollbar to trigger the ScrollBox.ScrollBy event which is the procedure to keep the arrow button to be  always visible after scrolled up/down.
-----------

What do you think ?
« Last Edit: April 30, 2014, 09:41:52 am by x2nie »
When you were logged in, you can see attachments.
Lazarus Github @ UbuntuCinnamon-v22.04.1 + LinuxMintDebianEdition5

taazz

  • Hero Member
  • *****
  • Posts: 5368
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #55 on: April 30, 2014, 09:26:10 am »
It looks good to me then again the delphi one looked good to me too so....
Can you set the arrow button on the right to bottom aligned as well it seems out of place as it is now looks like a top heavy item balancing on a pin thin leg ready to fall at any time.
Good judgement is the result of experience … Experience is the result of bad judgement.

OS : Windows 7 64 bit
Laz: Lazarus 1.4.4 FPC 2.6.4 i386-win32-win32/win64

x2nie

  • Hero Member
  • *****
  • Posts: 515
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #56 on: April 30, 2014, 10:01:34 am »
Can you set the arrow button on the right to bottom aligned as well it seems out of place as it is now looks like a top heavy ...


Sure I can do that. You are talking about the Chevron.
In theory, it would be easier by set Align := alClient
because it stay in a TPanel and this panel Align := alRight


see the attachment. Do you like it?
But it's not the Lazarus in action, the picture below is a mockup made using PaintBrush

When you were logged in, you can see attachments.
Lazarus Github @ UbuntuCinnamon-v22.04.1 + LinuxMintDebianEdition5

x2nie

  • Hero Member
  • *****
  • Posts: 515
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #57 on: April 30, 2014, 10:20:01 am »
Can you set the arrow button on the right to bottom aligned ..


Oops, bottom aligned?
IMHO center aligned is better.


But, okay...
Let we compare the above mockup with below one.



When you were logged in, you can see attachments.
Lazarus Github @ UbuntuCinnamon-v22.04.1 + LinuxMintDebianEdition5

taazz

  • Hero Member
  • *****
  • Posts: 5368
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #58 on: April 30, 2014, 10:35:51 am »
on your 2 lines height mock up does not make a big difference if centered or bottom although I prefer bottom on this too but here is a mock up from your 2nd screen shot and the real difference between center and bottom. I still think that bottom is better although I don't like it there at all.

But this is your enhancements, thank you for your time, you should choose based on what you like not me.
Good judgement is the result of experience … Experience is the result of bad judgement.

OS : Windows 7 64 bit
Laz: Lazarus 1.4.4 FPC 2.6.4 i386-win32-win32/win64

x2nie

  • Hero Member
  • *****
  • Posts: 515
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Re: Aesthetic bugfix of Lazarus's Component Palette
« Reply #59 on: April 30, 2014, 10:45:00 am »

Martin, I think I found the solution including for GTK2 or any other widgetset...
Quote
Code: [Select]
    if Rows > 1 then       //multi line?
       Dec(MaxBtnPerRow);  //reduced.

Not always correct.

1) In theory on some widget, scrollbar can take more space than one item.

2) if item is 20 pixel width, and scroll bar 10, and total 55, then you need not decrease it at all.

-----------------
The bug in gtk needs to be fixed.
Using ClientSizeWithoutBar  is the correct calculation.

So that will be kept.


If the problem is calculation result of scrollbar width, so..
Can we use it to calculate the vertical scrollbar width : ?
Code: [Select]
LCLIntf.GetSystemMetrics(SM_CXVSCROLL);
LCLIntf.GetSystemMetrics(SM_CXVSCROLL);


as LCLIntf.GetSystemMetrics is widely used in the IDE source code, it seem we can believe it as already implemented by widgetset.

When you were logged in, you can see attachments.
Lazarus Github @ UbuntuCinnamon-v22.04.1 + LinuxMintDebianEdition5

 

TinyPortal © 2005-2018