Mandag 20 juni 2011 22:37:10 skrev ja...@gmx.net: > On Monday 20 June 2011 15:31:28 Dag Andersen wrote: > > Mandag 20 juni 2011 15:20:27 skrev du: > > > On 20.06.2011 13:02, Dag Andersen wrote: > > > > Mandag 20 juni 2011 11:37:13 skrev du: > > > >> On 20.06.2011 10:34, Dag Andersen wrote: > > > >>> Before the weekend I fixed a crash in ArtisticTextShape and, as Jan > > > >>> Hambrect pointed out, I unvittingly removed the functionallity to > > > >>> go direct between shapes. So I thought I'd find out how this > > > >>> *should* be implemented before I tried to put it back, but I > > > >>> can't! > > > >>> I can find code that afaics is supposed to do it, but it is never > > > >>> executed. It may be that it is just me being confused so I need to > > > >>> run this by somebody that is more familiar with the code than I am. > > > >>> > > > >>> There is two "types" of switches that can occure: > > > >>> 1) Switch between shapes with different tools. > > > >>> E.g. TextTool -> ArtisticTextTool. > > > >>> 2) Switch between shapes using the same tool. > > > >>> E.g. TextTool -> TextTool > > > >> > > > >> 3. Switch between shapes using a layer docker like in Karbon or > > > >> Stage. > > > > > > > > Hmm, how is this different from 2? > > > > > > The tool itself is not involved in changing the shape selection. > > > > > > >> This was the use case targeted in the artistic text tool. So the > > > >> user could just click on another artistic text shape from the layer > > > >> docker and have the tool use that for further editing. > > > > > > > > I opened karbon, added an artistic text shape, added a new layer and > > > > added a new artistic text shape to that layer. Is this the case you > > > > are thinking of? > > > > > > No, rather you have two artistic text shapes. While editing the first > > > one with the artistic text tool, you click on the second one inside the > > > layer docker. The clicked shape gets selected, the first shape gets > > > deselected. The tool gets a signal that the shape selection changed and > > > checks if the artistic text shape it is currently editing is still > > > selected. It notices that it is not selected anymore so it checks if > > > any other artistic text shape is selected. Yes there is one, so the > > > tool changes its internal state to editing of that newly selected > > > shape. > > > > Yes, just found out what you meant. > > I still maintain that the shape should not need to listen to the seletion > > changed signal, all switching should be handled by KoToolManager via > > activate()/deactivate(). > > If this is left to the shapes *all* shapes have to implement the same > > code... > > I am not sure that switching tools on each selection change is the right > thing to do. That would mean that you are losing the tool state each time > the selection changes. Well, the tool is only deactivated, not deleted so you can certainly save any states. But: I think I would expect to get back to the state I was in last time I edited a certain *shape*, so imo any states should be saved to the shape. Also, in your case, think of this: You have 1 text shape (ts) and 2 artistic text shapes (as). Now navigating as->as will give different result than as->ts->as. As a user I think I'd be confused.
I got a bit deeper into the code, and I realized that the primary criteria for tool activation is which shapes are selected. Hence, any tool activation should be triggered by selectionChanged(). Secondary criteria is if a tool is active and if it is the interaction tool or not. So I put this logic into KoToolManager::selectionChanged(), removed switching from currentLayerChanged() and KoToolProxy, and suddenly things started to work pretty well. I certainly think this is the way to go. If you think you need to be able to switch shapes while your tool stays active (use case?), I think we need to add a swicthShapes method to the tool, and enhance the switching logic in KoToolManager::selectionChanged(). -- Mvh. Dag Andersen _______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
