[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: [PATCH] kword table col insertion
From:       Thomas Zander <zander () planescape ! com>
Date:       2002-05-13 19:15:02
[Download RAW message or body]

On Mon, May 13, 2002 at 01:07:33PM +0200, Emmanuel Touzery wrote:
> Hello,
> 
> 	The attached patch allows to insert a new column in a kword table even if
> there is no more space on the page on the right of the table. the table will
> simply be resized before adding the column. additionnally, the column size is
> then "smart", which means that if you had a two-columns table that took the
> whole page width (default when you create a table), you'll get a nicely
> proportionned three columns table (with three columns of the same width of
> 1/3rd of the page).
> 	the function "resizeWidth" in the kwtableframeset might be not so good, i
> don't know the code for tables that well. also maybe you won't like the
> arrangement of the code (???).
> 
> works here (tm), hope this will be better than my previous patch ;O)
> 
> emmanuel.
> 
> PS: note that this patch is against one pre-beta1 (JUST before beta1) 
> CVS. for now i don't have internet at home so i was doing this on this older 
> checkout (brought my HD at work). i won't have a proper connectivity at home 
> before some time so if it doesn't apply cleanly and you have some time it's 
> good if you could merge it in...
> is it ok if i send a couple of patches on an "older" CVS?
Patching was not that big a problem; just some fuzz. So as long as it works,
I wont make any problems :)

> PPS: posted first on master.kde.org, now on kde.org?
Yes; I got the email 5 times.


Looks great; good idea. There are however some little problems with it.

First of all, it does not compile :)
m_sDefaultColWidth is used in the command; while it is called col*size*
also;
 m_sDefaultColSize is added to the framesetEdit in the .h file but used 
on the frameset..
I also think that you would be better off by using the 60 as a default in 
the method definition instead of using 0 and then setting it to 60 if it
happened to be 0.
Just use the minFrameWidth const to assert it will not be too smal.

For inline frames the command won't work; the width is that of the parent
frame, not of the width of the page.
Something like:
 if(table->isFloating())
    width = table->anchorFrameset()->frame(0)->width();
is needed here.

I hate to ask people who actually contribute code to please use the KOffice 
default of indenting, it feels like I don't appriciate your stuff..
But please use spaces and not tabs.. Please? :)

You use a border of 2 pt in the kwcommand; but the borders are all calculated
internally in the kwtableframeset and only used when positioning the cells.
In other words; why do you add it?

Also take a look at the KWCanvas::createTable() in relation to creating
new frames. The frames have an incorrect zOrder making then new cells appear
below the main frameset.
You can probably just copy the zOrder of an existing cell (on the same page!)

Thanx!
-- 
Thomas Zander                                            zander@earthling.net
                                                 We are what we pretend to be

[Attachment #3 (application/pgp-signature)]
_______________________________________________
koffice-devel mailing list
koffice-devel@mail.kde.org
http://mail.kde.org/mailman/listinfo/koffice-devel

[prev in list] [next in list] [prev in thread] [next in thread]