[Libreoffice] [PATCH] conditional formatting with an unlimited number of rules

Robert Dargaud libo at bobiciel.com
Sun Apr 10 16:09:00 PDT 2011


Hi Katarina & Christoph (& All),

I have improved code like you suggest.

Le 06/04/11 17:56, Katarina Machalkova a écrit :
> 1) svx part of the patch is mostly OK ... except for the long list of new
> #includes. As the (French) comment says, not all of them are needed

added class name in comment and deactivated un-used #include (but can be 
used in futur for other attributes preview like border, rotation, ...)

> 2) insert

see below, in Christoph's text

> 3) In ScConditionalFormatDlg::InsertEntry and ScConditionalFormatDlg::AddEntry
> -- instead of handling/copying array of pointers to conditional format entries
> in this way, it'd be cleaner to work with some C++ container holding the
> pointers to entries -- std::vector or boost::ptr_vector

I will work on this improvement a next time.

> 4) In ::Refresh and ::UpdateValueList (as well as ::CondNChecked) methods
> really, really lot of code is duplicated/copy&pasted around. That's on one
> hand consistent with the rest of the code in this class (yeah, unfortunately),
> but on the other hand not very nice.

The code is small improved, but I think that it will be necessary to 
rewrite the entire class. I will work on it later.

> 5) Found a small buglet while testing (easy to fix) -- for newly appended
> condition entries, pre-selected style should be "Default". With your patch, it
> is the one that comes first in alphabetically sorted list (e.g. if I create a
> custom style named "Blue", it'll come before "Default" and all the newly
> created rules will have "Blue" style as default)

ok, it's fixed


Le 08/04/11 11:38, Christoph Noack a écrit :
> Hi Bob, hi Katarina, all!
> 	
> [...]
> Quick fix:
>        * Please add new entries always at the end of the list
>        * Scroll down to this item automatically (after the button has
>          been pressed and the new entry has been added)
>        * Add new items without being checked in the check-box (good
>          proposal from your side!)
>
> Furthermore, the name "Insert" might be confusing - it doesn't tell
> where the item will get added. Furthermore, the "1990s dialog
> layout" (vertical buttons) conflicts with the new functionality. So ...
>
> Quick fix:
>        * Rename the "Insert" button to "Add"
>        * Move the "Add" button below the "Help" button. -->  New order:
>          OK, Cancel, Help, Add (a bit distance between Help and Add would
>          be good)

I've removed the "insert" functionality until I find a better solution

and added a "Add" button exactly like asked

>> (I hope my googled-english is readable ;-)
> Yep. And hehe, I hope my Christoph-English is readable as well ;-)))

:-)

> [...]
> I really hope this helped a bit ... and makes the feature really great
> for users. So if you have any comments, questions, ... please tell me.

Ok, thank for your help

> And - of course - I also have some ideas how to really improve the
> dialog and bring it to the "2010s" :-) But this may require a bit more
> work for all of us :-)

Me too, I've some ideas : It should be between the simplicity of Apple 
Number and the gaz factory of MSO 2010 ;-)

But in first time, I wanted to propose a solution that does not disrupt 
the current user interface.

Best regard

Bob


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-Conditional-formatting-with-unlimited-numb.libs-core.patch
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20110411/ef47b138/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Conditional-formatting-with-an-unlimited-r.libs-core.patch
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20110411/ef47b138/attachment-0002.asc>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-Conditional-formatting-with-unlimited-number-of.calc.patch
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20110411/ef47b138/attachment-0002.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Conditional-formatting-with-an-unlimited-rules-.calc.patch
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20110411/ef47b138/attachment-0003.asc>


More information about the LibreOffice mailing list