D19216: Karbon: Enable multi page capability

2019-02-25 Thread Dag Andersen
danders added a comment.


  In D19216#417502 , @rjvbb wrote:
  
  > > Canvas color:
  > > I don't quite see what it is for. You can set a background color for the 
canvas but it is only for the views, it is not printed.
  >
  > A custom canvas colour feature doesn't strike me as odd, nor that it isn't 
printed (printing it WITHOUT setting a dedicated option would seem wrong to me).
  >
  > What about when you use an inversed theme, isn't a custom canvas colour 
required then if you want to see your black line art on a light (white) canvas?
  
  
  No, the canvas is part of the document and must never be themed. The canvas 
background is as much part of your drawing as any line you put on it.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D19216

To: danders, anthonyfieroni
Cc: rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever


D19216: Karbon: Enable multi page capability

2019-02-25 Thread Dag Andersen
danders added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in KarbonConfigInterfacePage.cpp:73-90
> We can remove "number of recent files" since other apps does not have it. but 
> for other 2 i don't see why we remove them, someone can found useful 
> (somehow). About me, i want them back, recent file complete remove not 
> commented.

I reconsiddered this, sheets actually also implements this so I'll leave it in.
(It does not work properly neither for karbon nor sheets, but that for 
different rainy day)

> danders wrote in KarbonConfigInterfacePage.cpp:73-90
> Recent file & docker font:
> Why does karbon need these, none of the other apps have it.
> Personally I would remove it, alternativly implent for all apps.
> Canvas color:
> I don't quite see what it is for. You can set a background color for the 
> canvas but it is only for the views, it is not printed.
> Also, if you have multiple views, it sets it in all views.

I also reconsidder docker font setting. It is useful and (partially) 
implemented in libs. You have to restart to get the new setting.
This should be impleneted for all apps, but that's for a different patch.

But, when it comes to canvas background, the more a look into it the more I 
dislike it.
Afaics it goes against both odf spec, wysiwyg and pageapp implementation.
Unfortunatly it was implemented so long ago that there is not mail archive any 
longer and
the git log doesn't give any reason.
Unless I get a *very* good reason to implement this I will not include it.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D19216

To: danders, anthonyfieroni
Cc: rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever


D19216: Karbon: Enable multi page capability

2019-02-25 Thread Camilla Boemann
boemann added a comment.


  totally agree about not theme'ing canvas
  
  Also a general agreement to do the page app thing as long as it's also 
supported in svg
  
  odg is hardly that much of a reason - it even seems like odf is moving away 
from odg as much as possible

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D19216

To: danders, anthonyfieroni
Cc: boemann, rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever


D19216: Karbon: Enable multi page capability

2019-02-25 Thread Dag Andersen
danders marked an inline comment as done.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D19216

To: danders, anthonyfieroni
Cc: boemann, rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever


D15428: [textlayout] Don't enter infinite loop when table is misfit

2019-02-25 Thread Dag Andersen
danders added a comment.


  Can we get a conclussion to this?
  @Camilla Have you come up with any more unit tests?

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D15428

To: anthonyfieroni, #calligra:_3.0, danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever


D15428: [textlayout] Don't enter infinite loop when table is misfit

2019-02-25 Thread Camilla Boemann
boemann added a comment.


  no

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D15428

To: anthonyfieroni, #calligra:_3.0, danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever


D19327: Karbon: Enable multi page capability

2019-02-25 Thread Dag Andersen
danders created this revision.
danders added a reviewer: anthonyfieroni.
danders added a project: Calligra: 3.0.
danders requested review of this revision.

REVISION SUMMARY
  Since odg spec supports multiple pages, I feel karbon also needs to support 
it.
  
  Ported to use pageapp classes.
  
  Karbon has config group "Interface" that is not precent in other apps.
  Some of the options have been disabled atm. Imho they should be
  harmonized with other apps and/or moved to View menu.
  
  A lot of code was duplicated between pageapp and karbon
  and has been removed from karbon:
  
  - Save/load
  - Layers docker and all layer operations
  - Grid, guides, rulers and zoom
  - Event handlers
  - Printing
  - Show page margins has been moved to pageapp
  
  In general, import/export needs review to determine how to
  handle multiple pages when e.g. exporting to a format that
  does not support pages.
  
  Known bugs:
  
  - "Separate paths" command:
- Execute command, the shape disappears.
- Undo crashes. Note: Afaics this code is not touched so probably a libs 
bug.
  
  - Snap to grid does not work
  
  - Number of recent files does not work properly

TEST PLAN
  I am not an avid user of karbon, so would be nice if some that where could 
test.
  Also, do not have all types of different format docs for filter testing.
  Some that work are pdf, svg, jpg and karbon files.

REPOSITORY
  R8 Calligra

BRANCH
  karbon_multipage_danders

REVISION DETAIL
  https://phabricator.kde.org/D19327

AFFECTED FILES
  filters/karbon/CMakeLists.txt
  filters/karbon/image/CMakeLists.txt
  filters/karbon/image/ImageExport.cpp
  filters/karbon/svg/CMakeLists.txt
  filters/karbon/svg/SvgExport.cpp
  filters/karbon/svg/SvgImport.cpp
  filters/karbon/wmf/CMakeLists.txt
  filters/karbon/wmf/WmfExport.cpp
  karbon/CMakeLists.txt
  karbon/data/karbon.rc
  karbon/ui/CMakeLists.txt
  karbon/ui/KarbonDocument.cpp
  karbon/ui/KarbonDocument.h
  karbon/ui/KarbonDocumentMergeCommand.cpp
  karbon/ui/KarbonDocumentMergeCommand.h
  karbon/ui/KarbonPart.cpp
  karbon/ui/KarbonPart.h
  karbon/ui/KarbonPrintJob.cpp
  karbon/ui/KarbonPrintJob.h
  karbon/ui/KarbonView.cpp
  karbon/ui/KarbonView.h
  karbon/ui/ProxyView.cpp
  karbon/ui/ProxyView.h
  karbon/ui/dockers/KarbonLayerDocker.cpp
  karbon/ui/dockers/KarbonLayerDocker.h
  karbon/ui/dockers/KarbonLayerModel.cpp
  karbon/ui/dockers/KarbonLayerModel.h
  karbon/ui/dockers/KarbonLayerSortingModel.cpp
  karbon/ui/dockers/KarbonLayerSortingModel.h
  karbon/ui/widgets/KarbonCanvas.cpp
  karbon/ui/widgets/KarbonCanvas.h
  karbon/ui/widgets/KarbonConfigInterfacePage.cpp
  karbon/ui/widgets/KarbonSmallStylePreview.h
  libs/pageapp/KoPAView.cpp
  libs/pageapp/KoPAView.h

To: danders, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever