Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar

Hi Anurag
On 26/06/11 18:23, Anurag Jain wrote:
> Hello Noel,
>
> On Fri, Jun 24, 2011 at 7:53 PM, Noel Power<nopower@novell.com>  wrote:
[...]
> I'm sending the fix patch here which calculates the height and width
> of the ScInputbarGroup and makes the border invisible.
great, thanks for that, more comments about the patch below
> There is no class named as ScTextWindow
forgive my typo, I meant ScTextWn
> > I suggest this because it is intended when the gsoc code is initially
> > integrated that it be configurable and for this reason it probably makes
> > sense to preserve the original ScTextWindow class and distinguish it from
> > your new implementation. This will make maintenance easier.
> So I guess there should not be any problem with the maintenance of
> code as I've not renamed any pre existing class and just inserted a
> new class i.e ScInputBarGroup.
well, true you have created a new class but additionally you have 
modified the existing class ScTextWnd, I suspect you will need to modify 
that class even more which is why I suggest that you rename the current 
version of ScTextWnd ( e.g. the one with your modifications ) to 
ScSomeNewName and restore the old ScTextWnd class. That way the old 
class is available to swap in at runtime and all ( or nearly all ) your 
modifications are in new classes.
> > Then next thing to concentrate on is getting the multi-line textbox
> > functionality to work and to make that behaviour switchable. I believe Kohei
> > suggested adding a button to the ScInputBarGroup window. So, you need to be
> > able to detect the button click and switch the state of the bIsMultiLine
> > flag that you have already added ( presumably for this purpose ) please do a
> > grep for PushButton&  SetClickHdl in sc for sample code for handling the
> > button.
> Next hurdle which I'm trying to get through is getting the Scrollbar
> and Button at the right position after the Textbar ends. Does the
> function SetPosSizePixel() or SetPosPixel() in window.hxx can be used
> for getting them at the desired position.
yes, you will need to set the position of the controls within the window 
manually by the methods you mention
>   Or is there any other more
> modular way to decide the ordering of these items similar to
> InsertWindow() and InsetItem() methods used in ScInputWindow ?
I don't think so, afaik the methods you mention are specific to a 
toolbar which has it's own custom layouting
> > When you get a button click you will need to resize your InputBarGroupbox
> > window based on the new state of the bIsMultiLine flag and of course
> > increase the height in multiples of the calculated height of a single line.
> > e.g. in Multiline mode the ScInputGroupBox height should be some multiple of
> > the calculated singleline height ( maybe 10 as a default ? ) Of course the
> > size of the ScMultiLineTxtWin (let's call it that for now but please rename
> > it as you wish ) window needs to also be adjusted and everything resized. I
> > would do this first before worrying about scrollbar behaviour.
> yes as soon as I get things placed in right place I'll be working on
> switching from single line to multiline mode on the button click.
please just implement the button and the multiline mode switching for 
the moment, adding the scrollbar to the mix adds more complexity and I 
would prefer to get the core functionality working first. Then you can 
build on that existing functionality to add more 'features'. Of course 
if you get the button and multiline-mode switching working then by all 
means start looking at the scrollbar part.

now, about the patch...

a) so, like mentioned above probably better to not enable those 
scrollbar drawing bits for the moment
b) Toolbox::Resize(), in line with the comments above about minimizing 
changes to the existing codebase by the multimode changes I'd prefer if 
the ScInputBarGroup class would take care of it's own resize. Currently 
how it's done doesn't make a whole lot of sense e.g. Toolbox::Resize 
manually calculates the size of the ScInputBarGroup, sets it's size then 
calls ScInputBarGroup::Resize, that just doesn't feel right. I think 
ScInputBarGroup::Resize should just calculate it's size ( taking 
whatever values from the parent window as needed )
c) at the moment ScInputBarGroup::Resize() is setting the aTextWindow's 
height to 22, I don't think the size of aTextWindow's height should be 
hardcoded to a number, looking in the code for ScTextWnd ( or whatever 
you rename it to be ) it already calculates the height ( single line 
height I guess ) I think it would make sense add a method to 
ScTextWndRenamedToSomething like 'GetSingleLineHeight' and then resize 
the ScInputBarGroup & ScTextWndRenamedToSomething  based on the number 
of lines you want to display, e.g. 1 * 
ScTextWndRenamedToSomething.GetHeight() for single line mode, and 'N' * 
ScTextWndRenamedToSomething.GetHeight() for multiline mode.

hope that makes sense :-)

Noel