Re: [Libreoffice] [GSOC][patch] Multiline inputbar
Hi Anurag, On Tue, 2011-08-23 at 20:25 +0530, Anurag Jain wrote: > Hello Kohei, > > This is a fix to some of the things you mentioned in last patch. > Setting the paper size inside the Resize() of ScMultiTextWnd solves > two issues > > 1: Prevents premature wrapping which was happening before. > 2: The paperSize is recalculated as per the window size. Thanks for your patch. Before I apply your patch, could you confirm that your patch is submitted under LGPLv3+/MPL 1.1 licenses? Actually, confirming that for all your previous patches would be great. I should've asked that before, but I forgot. :-P Thanks a lot, -- Kohei Yoshida, LibreOffice hacker, Calc ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][patch] Multiline inputbar
Hi Anurag, On Sat, 2011-08-20 at 23:16 +0530, Anurag Jain wrote: > Hello Kohei, > > I've Worked around ScrollBar and Made it appear. But still not able to > get the event handling thing working. Also I've improved some previous > code which handles sizing and positioning of button and other > controls. Also in order to delay the call to InitEditEngine, I think > we can make use of the GainFocus() method. The call should be made > only after the textbox has got focus. Firstly, I want you to guide me > in syncing the ScrollBar event with the textbox and after that I'll go > for fixing the EditEngine thing. So, the first thing I would try is to set the size of the scroll bar correctly to align it with the top and bottom ends of the input box to the left. Incidentally, the scroll bar should be located to the immediate right of the text box and to the left of the button, to make it appear as part of the text input box. Once that's done, go find out how you can get notified of changes with the line count, current line etc from the EditEngine implementation because you need to adjust the scroll bar range and position based on that. I'm not really up-to-date with the EditEngine code enough to tell you exactly what methods to use etc. so you need to do some digging into the EditEngine code to find out. > I've attached the patch over here, awaiting your suggestions. I've pushed your patch to the feature branch. One thing I noticed, now I see a black box in cell A1 when Calc starts up, which is probably due to the delaying of the edit engine initialization. You commented out the call to InitEditEngine() in the Paint() method. So, you need to somehow find a way to avoid this painting issue. My suggestion is to, instead of delaying the initialization of EditEngine in order to avoid incorrect size to be set to the controls, try calculating the size of the controls before initializing the edit engine to see if that works. Another issue I found. once the edit engine size is set to a certain size, even after resizing the application window, the edit engine size will remain the same. We need to have this fixed. In theory, we need to reset the size of the edit engine whenever the size of the input box changes, in order to avoid horizontal scrolls. Anyway, technically, today is the firm pencil-down date, so I will start evaluating your work based on what we have today. But if you need to give me more updates, feel free to send me patches daily. I'll probably start my evaluation sometime mid-week and I'll use whatever code is available that time to do a final evaluation. Good stuff! Kohei -- Kohei Yoshida, LibreOffice hacker, Calc ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][patch] Multiline inputbar
Hi Anurag, I pushed your patch to the feature branch. I think I noticed is that your commit name is all caps ANURAG JAIN. Could you fix that to properly capped name for your future patches? On Thu, 2011-08-04 at 13:43 +0530, Anurag Jain wrote: > I'll implement these things after returning from college. Also I'd > like to know how to go about the next step.? Is it going to be > scrollbar implementation or working on refreshing the windows ? Let's do scrollbar implementation first & get that working. The refreshing issue turns out to be pretty complex, so I'm not sure if we have enough time to fix that in your remaining GSOC time. But if you can implement the scrollbar and still have time left, let's look into that. Regards, Kohei -- Kohei Yoshida, LibreOffice hacker, Calc ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][patch] Multiline inputbar
Hi Anurag, On Mon, 2011-07-11 at 08:28 +0530, Anurag Jain wrote: > 1: Setting the proper output area when the toggling happens. I've > tried working on it but was not able to get vertical offset proper > when in multiline mode. So need to look more into its mathematics. > > 2: As Noel mentioned in, his last mail that I'm setting the height to > TBX_WINDOW_HEIGHT instead it can be more than that. I tried fetching > the current height in ScMultiTextWnd::Resize() and setting the correct > height but I did some wrong implementation and LO was crashing because > of that. So I'd need to look into that too. > > 3: The third thing is, when the resize happens, the ScInputWindow i.e > toolbox doesn't resize. The overflow of the child window is hidden > inside ScInputWindow. We already talked about the next step in IRC, so this is just a formality. We are now focusing on resolving item 3 since without it it's hard to resolve the other issues. ScInputWindow is not resizing on button push because we are not resizing it. We need to explicitly resize ScInputWindow on button press since there is no automatic resizing of any sort with the VCL controls. When ScInputWindow gets resized, it will in turn resize its immediate child control ScInputBarGroup, which will resize ScMultiTextWnd, in this order. Let's get that resolved first, before looking into the other issues. Meanwhile your patch has been committed & pushed to the feature branch. Kohei -- Kohei Yoshida, LibreOffice hacker, Calc ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
On 05/07/11 03:52, Anurag Jain wrote: As of now there are might be some printf's lying around so ignore them as of now. Awaiting your feedback on this. Ok, here goes 1) ScInputBarGroup::ScInputBarGroup() In the initialization list, bIsMultiLine( this ) is wrong surely, bIsMultiLine is of sal_Bool type, and there is some weird behaviour associated with that when you first use the input box. 2) ScInputBarGroup::ScInputBarGroup() also aButton ( this, ScResId( 1 )), I doubt this is correct, not sure what ScResId( 1 ) corresponds to but surely it initialises either text or some graphic associated with the button, in your case you have neither, best to not pass that unless we specifically define or reuse some resource for the button 3) ScInputBarGroup::Resize() if(bIsMultiLine) aSize.Height()=3*TBX_WINDOW_HEIGHT; else aSize.Height()=TBX_WINDOW_HEIGHT; can you indent that properly please 4) ScInputBarGroup::Resize() you set the height of the ScInputBarGroup to a the default toolbar window height or a multiple of the toolbar window height depending on the mode. It makes more sense to set the size to the size of the height the textbox will be for the mode ( the textbox height in single line mode is determined by the TBX_WINDOW_HEIGHT or the minimum edit height ) by exclusively using TBX_WINDOW_HEIGHT you are ignoring the fact the preferred height of the window could be larger than TBX_WINDOW_HEIGHT. But... we discussed the previously 5) ScInputBarGroup::Resize() I don't like magic numbers, e.g. the '10' in aSize.Width() = Max( ((long)(nWidth - nLeft - 10)), (long)0 ); it does nothing for the readability and certainly doesn't make it easier to understand. I suppose it is to make to leave a little space between the rightmost edge of the toolbar and the InputBarGroup, if so a #define or const value with some some sensible name would be in order. Since the windows is effectively invisible not sure if leaving that space really gains anything. Again this was something we talked about before 6) ScInputBarGroup::Resize() same thing with aButton.SetPosSizePixel(Point(aSize.Width()-40,0),Size(15,22)); make some sensible constant for 40 because that same number must be used where you resize the actually textwindow the button is to sit next to right ? also isn't is 40 a little large ? 7) ScInputBarGroup::Resize() the button dimensions, again these are hardcoded, iiwy I would make the height of the button the same as the non-multiline height of the textbox , I would make the width some fraction of the height ( that gives it a pleasant aspect ). 8) ScInputBarGroup::Resize() I see some use of Invalidate here ( and elsewhere ), I have to say I am not sure what the real purpose of Invalidate is, presumably it ensures that a window is marked dirty for the purposes of repainting or resizing or whatnot ( at least I have seen previously calling Invalidate eventually calls Resize ) But... if there is a preferred way of using it e.g. call SetSizePixel on a window then call Invalidate etc. I don't know, maybe someone else might about Resizing in general, to me the way you seem to be daisy-chaining the resizing seems to make sense ( not sure from a vcl point of view if it does ) but at lest it is easy to follow, Toolbar:Resize calls InputBar::Resize which calls TextBoxThing::Resize. If it is wrong then at least a logical arrangement should be easy to reorganise 9)IMPL_LINK( ScInputBarGroup, ClickHdl, PushButton*, pBtn ) the check for bIsMultiline to set it e.g. if(!bIsMultiLine) { bIsMultiLine=true; aTextWindow.SetMultiLineStatus(true); Resize(); } else { bIsMultiLine=false; aTextWindow.SetMultiLineStatus(false); Resize(); } is a little long winded, just assign bIsMultiLine to it's logical NOT value and then call the following lines aTextWindow.SetMultiLineStatus(bIsMultiLine); Resize(); no need to duplicate the above lines 10) ScMultibar::ScMultibar the order of initialisation of bInputMode & bIsMultiLine is incorrect ( see warnings when you compile ) there are some other you introduced too, please fix them up too if they still exist after merging you code with the updated feature-branch 11 ) ScMultibar::Resize hmm setting the size of the text box to the toolbox height ( or multiple ) again here just seems wrong. Personally I would determine the height to be a the normal single line mode height that is normally determined in the constructor of this class, in multiple-line mode I would make that number to be a multiple of that height. so InputBarGroup::Resize would probably determine it's own height dependant on the preferred height of the textbox/ScMultiBar ( the naming is getting confusing here since I renamed this to ScMultiTextWnd ) - I will use ScMultiTextWnd in future 12) ScMultibar::Resize shouldn't the size of the output area also be changed, even though the size of the
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
On 05/07/11 03:52, Anurag Jain wrote: Hello Noel, As you asked I'm sending a patch here with most of the changes incorporated. I've done total rework again and made a bit more modular, removed unnecessary things and now it look way better than what i was getting yesterday. Please have a look before you make changes on you end and if you think this is good enough to be pushed into Master please do so. Sorry but it's a too late I have vacation Wed/Thur ( not sure when Kohei is back ) and the integration into master needs to be done by Thursday. I expected this patch much earlier ( by last Friday we agreed on IRC ) and and now time is too short. I already told you on IRC that I was integrating your code. It is now integrated on master and also for your convenience on the feature branch, see http://cgit.freedesktop.org/libreoffice/calc/commit/?id=971ff8c26b8d9559367a8b86fbf967327cbd70e0 Summary of the changes a) Your code is now runtime switchable, by default the code runs as before, to enable your multiline input work then from libreoffice use Tools | Options | General | Enable experimental feature. No need to restart libreoffice and newly opened calc document will honour the setting b) Made ScInputBarGroup and ScTextWnd substitutable by giving them a common base class, added the code to swap using the configure switch above c) Since I never got the patch to restore the old ScTextWnd code or create the new class to capture the differences between your version of ScTextWnd & the original one I had to do this myself. I notice in the file you sent yesterday and the patch today that you still made a copy of the complete class :-( When I integrated your code I just inherited from ScTextWnd and captured the changes that way. Also I didn't like ScMultiBar so I renamed it to ScMulitTextWnd, I am not thrilled with the new name but I am happier with it than the other name d) for safety disabled the old accessibility related code when using the 'new' inputline stuff Note: it would be worth you checking that it runs as it should, I could have missed something ( although it seems to work as before ) I guess I'd be better if you make the runtime decision thing above this patch. ideally it would have been better for you to do this but it seems I failed to be able to get you to understand what was required :-( As of now there are might be some printf's lying around so ignore them as of now. Awaiting your feedback on this. I already explained to you on IRC that you will have to merge your changes into the new code base that resulted from me integrating your code. You at least need to do that for the feature branch. In the code integrated into master/feature-branch I haven't changed or cleaned up your code, just separated it from the core code so your modifications should be easy for you to port. Note: please back up your work before doing a git pull in the feature branch so you can refer back to your latest changes I will have a look at the patch as it stands and review it. Please try not to leave printf(s) in the code, use OSL_TRACE instead ( you need to do 'make dbglevel=2' to enable them). At least if an OSL_TRACE sneaks into master for example then normally it is compiled out and no unnecessary output will be sent to the terminal Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
Hello Noel, As you asked I'm sending a patch here with most of the changes incorporated. I've done total rework again and made a bit more modular, removed unnecessary things and now it look way better than what i was getting yesterday. Please have a look before you make changes on you end and if you think this is good enough to be pushed into Master please do so. I guess I'd be better if you make the runtime decision thing above this patch. As of now there are might be some printf's lying around so ignore them as of now. Awaiting your feedback on this. Thanks and regards On Mon, Jul 4, 2011 at 9:09 PM, Noel Power wrote: > On 04/07/11 11:29, Anurag Jain wrote: >> >> Hello Noel, >> >> I'm sending the inputwin.cxx with the changes I've made in >> ScInputBargroup. Do not push it as of now. I'd be making some more >> changes as mentioned in above mail before it can be pushed. > > I guess it really isn't usable without a header file :-( I really was > looking for a patch that addressed the issues we talked about previously > that would allow me to at least start to try and integrate your work. > >> On Mon, Jul 4, 2011 at 3:53 PM, Anurag Jain >> wrote: >>> >>> Hello Noel, >>> >>> Yes I have made some changes as you mentioned this weekend. There are >>> certain thing I'm not getting properly. >>> >>> 1: I'm able to toggle the height of ScMultiBar and ScInputBarGroup on >>> button click. But the increment happens inside the ScInputWindow i.e. >>> the excess window size do not appear. It toggles inside the >>> ScInputWindow. To make it look the height of complete panel should be >>> varied.So in order to do that will it be a good idea to add a >>> ScInputWindow object in ScInputBarGroup and use it to vary the whole >>> panel's height ? > > hmm, it seems that somehow the toolbar doesn't resize as expected, don't > know why, might be worth doing some debugging in > vcl/source/window/toolbox.cxx >>> >>> 2: Also one more thing, when button is clicked twice , the position >>> window, and other formula images all disappears leaving a blank on >>> left side of panel. I guess the ScInputBarGroup changes its vertical >>> position. I tried resetting it always to (nLeft,0) but the LO did not >>> start for some reason. As of now I'm trying to fix that. I'll discuss >>> about these problems today in IRC and things required to be improved a >>> little bit before putting it in master branch. > > no idea at all about that, it sounds like either a paint problem ( but > that's easy to check by forcing repaint but moving the window offscreen and > back ) or the controls have been repositioned somehow, again some debugging > of the size and position attributes of the toolbar items should give you an > idea > > I am going to try to integrate the code as it was before whatever new work > you have done because I there is little time left, it is better to have some > code integrated rather than none :-). I didn't look at the inputwin.cxx that > you attached in any great detail but I don't like the name of the new > textbox ScMultiBar, it is I think misleading, I liked some of the other > alternatives you suggested better ( e.g. ScMultiText ). > I know I suggested that you rename the class to something new but I thought > maybe you could reuse the old one ( e.g. inherit from it ) to avoid > duplicating *all* the code there. My main concern was to separate the new > from old code ( without excessive logic ( if/then ) branches in the code > especially with the requirement of having some code in master )... I will > try to fix that when integrating the code > > Noel > -- Anurag Jain Final yr B.Tech CSE SASTRA University Thanjavur(T.N.)-613402 diff --git a/sc/source/ui/app/inputwin.cxx b/sc/source/ui/app/inputwin.cxx index dc12376..ede8585 100644 --- a/sc/source/ui/app/inputwin.cxx +++ b/sc/source/ui/app/inputwin.cxx @@ -172,12 +172,10 @@ ScInputWindow::ScInputWindow( Window* pParent, SfxBindings* pBind ) : InsertItem ( SID_INPUT_EQUAL, IMAGE( SID_INPUT_EQUAL ), 0,4 ); InsertSeparator ( 5 ); InsertWindow( 7, &aBarGroup, 0, 6 ); -// InsertWindow( 8, &maScrollBar, 0, 8 ); +printf("Ctor ScInputWindow\n"); aWndPos .SetQuickHelpText( ScResId( SCSTR_QHELP_POSWND ) ); aWndPos.SetHelpId ( HID_INSWIN_POS ); -//aTextWindow.SetQuickHelpText( ScResId( SCSTR_QHELP_INPUTWND ) ); -//aTextWindow.SetHelpId ( HID_INSWIN_INPUT ); // kein SetHelpText, die Hilfetexte kommen aus der Hilfe @@ -193,8 +191,7 @@ ScInputWindow::ScInputWindow( Window* pParent, SfxBindings* pBind ) : SetHelpId( HID_SC_INPUTWIN ); // fuer die ganze Eingabezeile aWndPos .Show(); -// maScrollBar .Show(); -aBarGroup .Show(); +//aBarGroup .Show(); pInputHdl = SC_MOD()->GetInputHdl( pViewSh, false );// use own handler even if ref-handler is set if (pInputHdl) @@ -487,15 +484,22 @@ void ScI
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
On 04/07/11 11:29, Anurag Jain wrote: Hello Noel, I'm sending the inputwin.cxx with the changes I've made in ScInputBargroup. Do not push it as of now. I'd be making some more changes as mentioned in above mail before it can be pushed. I guess it really isn't usable without a header file :-( I really was looking for a patch that addressed the issues we talked about previously that would allow me to at least start to try and integrate your work. On Mon, Jul 4, 2011 at 3:53 PM, Anurag Jain wrote: Hello Noel, Yes I have made some changes as you mentioned this weekend. There are certain thing I'm not getting properly. 1: I'm able to toggle the height of ScMultiBar and ScInputBarGroup on button click. But the increment happens inside the ScInputWindow i.e. the excess window size do not appear. It toggles inside the ScInputWindow. To make it look the height of complete panel should be varied.So in order to do that will it be a good idea to add a ScInputWindow object in ScInputBarGroup and use it to vary the whole panel's height ? hmm, it seems that somehow the toolbar doesn't resize as expected, don't know why, might be worth doing some debugging in vcl/source/window/toolbox.cxx 2: Also one more thing, when button is clicked twice , the position window, and other formula images all disappears leaving a blank on left side of panel. I guess the ScInputBarGroup changes its vertical position. I tried resetting it always to (nLeft,0) but the LO did not start for some reason. As of now I'm trying to fix that. I'll discuss about these problems today in IRC and things required to be improved a little bit before putting it in master branch. no idea at all about that, it sounds like either a paint problem ( but that's easy to check by forcing repaint but moving the window offscreen and back ) or the controls have been repositioned somehow, again some debugging of the size and position attributes of the toolbar items should give you an idea I am going to try to integrate the code as it was before whatever new work you have done because I there is little time left, it is better to have some code integrated rather than none :-). I didn't look at the inputwin.cxx that you attached in any great detail but I don't like the name of the new textbox ScMultiBar, it is I think misleading, I liked some of the other alternatives you suggested better ( e.g. ScMultiText ). I know I suggested that you rename the class to something new but I thought maybe you could reuse the old one ( e.g. inherit from it ) to avoid duplicating *all* the code there. My main concern was to separate the new from old code ( without excessive logic ( if/then ) branches in the code especially with the requirement of having some code in master )... I will try to fix that when integrating the code Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
Hello Noel, Yes I have made some changes as you mentioned this weekend. There are certain thing I'm not getting properly. 1: I'm able to toggle the height of ScMultiBar and ScInputBarGroup on button click. But the increment happens inside the ScInputWindow i.e. the excess window size do not appear. It toggles inside the ScInputWindow. To make it look the height of complete panel should be varied.So in order to do that will it be a good idea to add a ScInputWindow object in ScInputBarGroup and use it to vary the whole panel's height ? 2: Also one more thing, when button is clicked twice , the position window, and other formula images all disappears leaving a blank on left side of panel. I guess the ScInputBarGroup changes its vertical position. I tried resetting it always to (nLeft,0) but the LO did not start for some reason. As of now I'm trying to fix that. I'll discuss about these problems today in IRC and things required to be improved a little bit before putting it in master branch. On Mon, Jul 4, 2011 at 3:29 PM, Noel Power wrote: > Hi Anurag, > As discussed on IRC I had hoped to see the patch you promised to send. The > one that rename's your class, restores ScTextWnd and removes the > unnecessary changes and does some cleanup of the existing code, is there > some problem? everything ok? I need those changes to integrate your code > into master. > On 26/06/11 18:23, Anurag Jain wrote: >> >> 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. >> > Is there any update regarding with size switching, any chance of a patch for > the above?, it would be great to get those changes in addition to the work > above into master ( would give people a better idea and feel for what you > are doing, without the resize changes the visual difference with the work > sofar isn't so obvious ) > So, time is fast running out on this, I need the patches already promised > asap, if you can provide the extra parts that's even better. > > Thanks > Noel > Thanks and regards. -- Anurag Jain Final yr B.Tech CSE SASTRA University Thanjavur(T.N.)-613402 ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
Hi Anurag, As discussed on IRC I had hoped to see the patch you promised to send. The one that rename's your class, restores ScTextWnd and removes the unnecessary changes and does some cleanup of the existing code, is there some problem? everything ok? I need those changes to integrate your code into master. On 26/06/11 18:23, Anurag Jain wrote: 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. Is there any update regarding with size switching, any chance of a patch for the above?, it would be great to get those changes in addition to the work above into master ( would give people a better idea and feel for what you are doing, without the resize changes the visual difference with the work sofar isn't so obvious ) So, time is fast running out on this, I need the patches already promised asap, if you can provide the extra parts that's even better. Thanks Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
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 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 ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
Hello Noel, On Fri, Jun 24, 2011 at 7:53 PM, Noel Power wrote: > Hi Anurag > On 22/06/11 15:51, Anurag Jain wrote: >> >> Hi Noel, >> >> Resending the patch again along with the source files here. > > as mentioned on Wed on iRC I pushed the patch to the feature branch. I had > hoped to see the latest changes you made to correct the control size problem > that I identified that was preventing your new control from being displayed > before reviewing the patch further, I am not *that* familiar with > libreoffice ui bits and pieces and was hoping to have something to run to > test. Additionally on IRC it wasn't clear how you now were calcuating the > the ScInputBarGroup width, I'd like to have seen the code for that > I'm sending the fix patch here which calculates the height and width of the ScInputbarGroup and makes the border invisible. > Anyway some comments on what is there so far, I think the ScInputBarGroup is > taking shape, clearly it's very rough at this stage ( understandable since > you had problems getting it to show ). One thing I would suggest here to > a) change the name of ScTextWindow to something like ScMultiLineTxtWin ( or > any other suitable name ) > b) restore the orig ScTextWindow class > There is no class named as ScTextWindow and never was. It has always been ScTextWnd-->ScInputWindow and now it has become like ScTextWnd-->ScInputBarGroup-->ScInputWidow > 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. > 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. Or is there any other more modular way to decide the ordering of these items similar to InsertWindow() and InsetItem() methods used in ScInputWindow ? > 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. > regards > Noel > > > Thanks and regards -- Anurag Jain Final yr B.Tech CSE SASTRA University Thanjavur(T.N.)-613402 diff --git a/sc/source/ui/app/inputwin.cxx b/sc/source/ui/app/inputwin.cxx index c3d0ffc..dc12376 100644 --- a/sc/source/ui/app/inputwin.cxx +++ b/sc/source/ui/app/inputwin.cxx @@ -140,7 +140,7 @@ ScInputWindow::ScInputWindow( Window* pParent, SfxBindings* pBind ) : ToolBox ( pParent, WinBits(WB_BORDER|WB_3DLOOK|WB_CLIPCHILDREN) ), aWndPos ( this ), // maScrollBar ( this, WB_VERT | WB_DRAG ), -aBarGroup ( this ), +aBarGroup ( this), pInputHdl ( NULL ), pBindings ( pBind ), aTextOk ( ScResId( SCSTR_QHELP_BTNOK ) ), // not always new from Resource @@ -179,14 +179,6 @@ ScInputWindow::ScInputWindow( Window* pParent, SfxBindings* pBind ) : //aTextWindow.SetQuickHelpText( ScResId( SCSTR_QHELP_INPUTWND ) ); //aTextWindow.SetHelpId ( HID_INSWIN_INPUT ); -/* -maScrollBar.SetPageSize( 1 ); -maScrollBar.SetVisibleSize( 1 ); -maScrollBar.SetLineSize( 1 ); -maScrollBar.SetRange( Range( 0, 1 ) ); -maScrollBar.SetThumbPos( 10 ); -*/ - // kein SetHelpText, die Hilfetexte kommen aus der Hilfe SetItemText ( SID_INPUT_FUNCTION, ScResId( SCSTR_QHELP_BTNCALC ) ); @@ -496,6 +488,15 @@ void ScInputWindow::Resize() { ToolBox::Resize(); +long nWidth = GetSizePixel().Width(); +long nLeft = aBarGroup.GetPosPixel().X(); +Size aSize = aBarGroup.
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
Hi Anurag On 22/06/11 15:51, Anurag Jain wrote: Hi Noel, Resending the patch again along with the source files here. as mentioned on Wed on iRC I pushed the patch to the feature branch. I had hoped to see the latest changes you made to correct the control size problem that I identified that was preventing your new control from being displayed before reviewing the patch further, I am not *that* familiar with libreoffice ui bits and pieces and was hoping to have something to run to test. Additionally on IRC it wasn't clear how you now were calcuating the the ScInputBarGroup width, I'd like to have seen the code for that Anyway some comments on what is there so far, I think the ScInputBarGroup is taking shape, clearly it's very rough at this stage ( understandable since you had problems getting it to show ). One thing I would suggest here to a) change the name of ScTextWindow to something like ScMultiLineTxtWin ( or any other suitable name ) b) restore the orig ScTextWindow class 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. 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. 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. regards Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
Hi Anurag On 22/06/11 05:20, Anurag Jain wrote: Hi Noel, Actually the problem started from not doing a git pull on my end after Kohei pushing my patch (created using git diff) as I went on making changes. In order to resolve this I ran git stash; git pull; and then git stash pop; And I made some more changes and sent you the same patch generated by git diff. no idea, all depends on what state your repo was in, what diff command you used etc. Where things would have gone wrong ? well I am no git expert either so I wont try and reconstruct what might have gone wrong Anyways since I'm not a git expert as you said to checkout a fresh SC; can you send me the git command which need to be run on my side. Or will a git reset Hard followed by pull will do the trick ? no, that wont work. the following command ( if I understand git correctly ) git diff origin/feature/calc-multiline-input should give a diff of your local state against the branch you are tracking But as mentioned earlier, there are ( afaik ) only 2 files you have modified, so please at least to be safe send inputwin.[ch]xx ( and any other files you modified ) in addition to the patch Thanks, Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
Hi Noel, Actually the problem started from not doing a git pull on my end after Kohei pushing my patch (created using git diff) as I went on making changes. In order to resolve this I ran git stash; git pull; and then git stash pop; And I made some more changes and sent you the same patch generated by git diff. Where things would have gone wrong ? Anyways since I'm not a git expert as you said to checkout a fresh SC; can you send me the git command which need to be run on my side. Or will a git reset Hard followed by pull will do the trick ? On Wed, Jun 22, 2011 at 12:57 AM, Noel Power wrote: > Hi Anurag > On 21/06/11 18:59, Anurag Jain wrote: > > Hi Noel, > > Please review the patch I'm sending here. I did not have luck getting > the control shown up. Please guide me what I'm missing ? > > Thanks and regards > > Sorry I think you need to redo the patch, it doesn't apply for me ( or at > least the hunks to inputwin.hxx don't ) And looking at it it's clear the > diff is not against the feature branch but some local version that you have > :-( > > for instance have a look at the final hunk and you can see there is a > ScInputBarGroup in the context ( but of course that doesn't exist yet ) > @@ -230,7 +229,6 @@ protected: > > private: > ScPosWnd aWndPos; > -// ScTextWnd aTextWindow; > ScInputBarGroup aBarGroup; > ScInputHandler* pInputHdl; > SfxBindings* pBindings; > > If I was you I would check out a fresh sc ( from the branch ) and do a diff > against your local files ( there are only 2 ) or alternatively send me the > inputwin.[ch]xx files > > thanks, > > Noel > > Thanks and regards -- Anurag Jain Final yr B.Tech CSE SASTRA University Thanjavur(T.N.)-613402 ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
Hi Anurag On 21/06/11 18:59, Anurag Jain wrote: Hi Noel, Please review the patch I'm sending here. I did not have luck getting the control shown up. Please guide me what I'm missing ? Thanks and regards Sorry I think you need to redo the patch, it doesn't apply for me ( or at least the hunks to inputwin.hxx don't ) And looking at it it's clear the diff is not against the feature branch but some local version that you have :-( for instance have a look at the final hunk and you can see there is a ScInputBarGroup in the context ( but of course that doesn't exist yet ) @@ -230,7 +229,6 @@ protected: private: ScPosWnd aWndPos; -// ScTextWnd aTextWindow; ScInputBarGroup aBarGroup; ScInputHandler* pInputHdl; SfxBindings* pBindings; If I was you I would check out a fresh sc ( from the branch ) and do a diff against your local files ( there are only 2 ) or alternatively send me the inputwin.[ch]xx files thanks, Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][patch] Multiline inputbar
On Tue, 2011-06-07 at 07:27 +0530, Anurag Jain wrote: > Hello Kohei, > > In this patch I've removed the hard coded values for the height Yup. This one looks much better. Pushed to your feature branch. :-) BTW, it would be nice to have you pull the new changes from the repository so that your next change will be against the most recent commit in the remote repo. So, please run cd sc git pull -r to get the latest changes before you make your next changes. This will make it easier for me to apply your patch in the future. Regards, Kohei -- Kohei Yoshida, LibreOffice hacker, Calc ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][patch] Multiline inputbar
Hi Kohei, Have a look into this. I've fixed the display problem with the j and g letters. On Tue, Jun 7, 2011 at 12:57 AM, Kohei Yoshida wrote: > Hi Anurag, > > On Tue, 2011-06-07 at 00:15 +0530, Anurag Jain wrote: >> Hello Kohei, >> >> I was able to figure out how to make the text appear properly in the >> inputbar when in single line mode. I'm sending my patch over here. >> Please have a look into it and let me know about further improvements >> that can be done. > > We talked a bit in IRC but just to let the list know... > > This change looks great! The text gets wrapped and the cursor moves to > the next line as the line reaches the full width of the input bar. And > the up/down arrow keys shifts the cursor to the previous/next line as > you would expect. Good work! :-) Thanks Kohei for the feedback and the motivation. ;) > > Now, a minor nit pick is that, the very lower portion of the text > appears to be cut off. For instance, when you type 'j', the lower > portion of the letter is not displayed and it looks like 'i'. Have you > tried EditEngine::GetTextHeight() ? That may give you a more > appropriate height to use rather than hard-coding it to the 1/4 of the > height of the input box. > Yeah this patch will fix the j and g thing. Also I'll remove the hard coding once you let me know about this patch. > I've checked in this change to your feature branch, though I didn't > check in those extra blank lines. > Yeah I'll take care of such things. > Kohei > > -- > Kohei Yoshida, LibreOffice hacker, Calc > > > Thanks and regards. -- Anurag Jain Final yr B.Tech CSE SASTRA University Thanjavur(T.N.)-613402 diff --git a/sc/source/ui/app/inputwin.cxx b/sc/source/ui/app/inputwin.cxx index db934bd..b77d7c4 100644 --- a/sc/source/ui/app/inputwin.cxx +++ b/sc/source/ui/app/inputwin.cxx @@ -787,19 +787,29 @@ void ScTextWnd::Paint( const Rectangle& rRec ) InitEditEngine(SfxObjectShell::Current()); if (pEditView) +{ + pEditView->Paint(rRec); + +} } + + void ScTextWnd::Resize() { if (pEditView) { Size aSize = GetOutputSizePixel(); -Point aPos(0, 0); +int count = pEditEngine->GetLineCount(0); +printf("%d %d\n", aSize.Height() , count); +//Point aPos(0,(count-1)*aSize.Height()); +Point aPos(TEXT_STARTPOS,4*aSize.Height()/22); +Point aPos2(aSize.Width()-5,18*aSize.Height()/22); // TODO : When in single line mode, set the height to the height of a // single line, and set the position so that the text look centered. pEditView->SetOutputArea( -PixelToLogic(Rectangle(aPos, aSize))); +PixelToLogic(Rectangle(aPos, aPos2))); } } ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][patch] Multiline inputbar
Hi Anurag, On Tue, 2011-06-07 at 00:15 +0530, Anurag Jain wrote: > Hello Kohei, > > I was able to figure out how to make the text appear properly in the > inputbar when in single line mode. I'm sending my patch over here. > Please have a look into it and let me know about further improvements > that can be done. We talked a bit in IRC but just to let the list know... This change looks great! The text gets wrapped and the cursor moves to the next line as the line reaches the full width of the input bar. And the up/down arrow keys shifts the cursor to the previous/next line as you would expect. Good work! :-) Now, a minor nit pick is that, the very lower portion of the text appears to be cut off. For instance, when you type 'j', the lower portion of the letter is not displayed and it looks like 'i'. Have you tried EditEngine::GetTextHeight() ? That may give you a more appropriate height to use rather than hard-coding it to the 1/4 of the height of the input box. I've checked in this change to your feature branch, though I didn't check in those extra blank lines. Kohei -- Kohei Yoshida, LibreOffice hacker, Calc ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
On Thu, 2011-05-12 at 23:27 -0400, Kohei Yoshida wrote: > On Fri, 2011-05-13 at 03:53 +0530, Anurag Jain wrote: > > Hello There, > > > > I'm posting here the git diff of the work I've done till now. > > @Kohei Since I do not want to disturb my master branch I'm here > > pasting the diff only. Please create a feature branch and apply it, so > > that I can clone that branch and commit on that. > > Could you re-send that patch as an attachment? I'm having hard time > applying that diff output because the mail client wrapped many lines > that basically screwed up the patch format. I have received a patch in private mail, and have checked it in to the feature/calc-multiline-input (the calc repo only for now). I have assumed that it is submitted under LGPLv3+/MPL. If not, let me know. Kohei -- Kohei Yoshida, LibreOffice hacker, Calc ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar
On Fri, 2011-05-13 at 03:53 +0530, Anurag Jain wrote: > Hello There, > > I'm posting here the git diff of the work I've done till now. > @Kohei Since I do not want to disturb my master branch I'm here > pasting the diff only. Please create a feature branch and apply it, so > that I can clone that branch and commit on that. Could you re-send that patch as an attachment? I'm having hard time applying that diff output because the mail client wrapped many lines that basically screwed up the patch format. Thanks, Kohei ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice