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

List:       koffice-devel
Subject:    flake/canvasController
From:       zander () kde ! org
Date:       2010-09-13 12:18:42
Message-ID: 201009131418.42785.zander () kde ! org
[Download RAW message or body]

On Monday 13. September 2010 09.51.57 Boudewijn Rempt wrote:
> SVN commit 1174707 by rempt:
> 
> Explain the design of KoCanvasController a bit more
> 
> (And yes, there is more than one implementation of KoCanvasController,
> it's just that not all of them are in the KOffice tree).

As flake is not yet frozen that is a dangerous proposition; I was already 
prepared to refactor the code to avoid the unneeded spitting of classes.

The docs give no indication why there is any need for a canvasController that 
is not based on a QWidget. And I can't come up with any.

As one of the maintainers of flake I would suggest that if some external 
project has this need there should not be a solution inside of KOffice like we 
have now; 3 classes that are split with no visible reason in any app I can 
find.

Please provide a link to the sources of this app so at minimum the flake 
maintainers can at minimum see what is going on and maybe write some better 
documentation on the logic and justification of this splitting.

I'm quite confused how this refactor managed to get through an API review, it 
goes against all the work we did in the last year in cleaning up this library.
If we ever want to get to binary compatibility, this kind of things should not 
slip!

Any suggestions on how to proceed?


>  M  +14 -10    KoCanvasController.h
> 
> --- trunk/koffice/libs/flake/KoCanvasController.h #1174706:1174707
> @@ -39,23 +39,27 @@
> 
>  class KoCanvasControllerProxyObject;
>  
>  /**
> 
> - * KoCanvasController is the base class for widgets that are a wrapper
> - * around your canvas providing scrollbars.
> + * KoCanvasController is the base class for wrappers around your canvas
> + * that provides scrolling and zoomming for your canvas.
> 
>   *
>   * Flake does not provide a canvas, the application will have to
> 
> - * extend a QWidget and implement that themselves; but Flake does make
> - * it a lot easier to do so. One of those things is this class which
> - * acts as a decorator around the canvas widget or graphics item and
> provides - * scrollbars and allows the canvas to be centered in the
> viewArea + * implement a canvas themselves. You canvas can be
> QWidget-based, QGraphicsItem-based + * or something we haven't invented
> yet -- as long the class that holds the canvas + * imlements
> KoCanvasController, tools, scrolling and zooming will work. + *
> + * A KoCanvasController implementation acts as a decorator around the
> canvas widget or + * graphics item and provides a way to scroll the
> cavasn, allows the canvas to be centered + * in the viewArea and manages
> tool activation.
> + *
> 
>   * <p>The using application can instantiate this class and add its
>   * canvas using the setCanvas() call. Which is designed so it can be
> 
> - * called multiple times for those that wish to exchange one canvas
> - * widget for another.
> + * called multiple times if you need to exchange one canvas
> + * widget for another, for instance, switching between a plain QWidget or
> a QGLWidget.
> 
>   *
> 
> - * Effectively, there is one KoCanvasController per KoView in your
> + * <p>There is one KoCanvasController per canvas in your
> 
>   * application.
>   *
> 
> - * The canvas widget is at most as big as the viewport of the scroll
> + * <p>The canvas widget is at most as big as the viewport of the scroll
> 
>   * area, and when the view on the document is near its edges, smaller.
>   * In your canvas widget code, you can find the right place in your
>   * document in view coordinates (pixels) by adding the documentOffset
-- 
Thomas Zander
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic