copy-paste-frames -- copy and paste frames inside Sophie

[deyan]

• Provide copy and paste of frames inside Sophie.
  • For frames with embedded resources - the resource should be copied.
    • If pasted in the same book, the resource and frame name should be with "Copy of" before the name
    • If pasted in different book, the resource and the frame name should be preserved.
  • For frames with linked resources - the link should be copied. This means that the link will point the same resource.
  • For paste
    • If pasted on different page, the location should be preserved
    • If pasted on the same page, the location should be with 10 px right and top offset.
• The copy-paste should be functional between two instances of Sophie

[deyan]

Batch update from file priorities.csv

[deyan]

Batch update from file report_1.csv

[deyan]

Batch update from file 0911261.csv

[deni]

• Main ideas:
  • When copying a resource (frame, group or page), it should be encapsulated, so that it contains as children all the other resources it depends on. For example, the main resource and the background image of a frame.
    • Not all ResourceRefR4s in the resource model should be deeply copied. For example, link targets should not.
    • Keys that contain resource refs that should be deeply copied will be annotated.
  • In order to provide the functionality of copying multiple resources, the resources being copied will be wrapped by a fake parent resource.
  • The resultant fake parent resource will be transferred by a ResourceTransferable
  • When pasting a resource, the ResourceRevisionData carried by ResourceTransferable will be used to load the fake parent resource and copy its children in the edit scope (current page or group).
    • This will ensure that resources are correctly copied in different books or different instances of Sophie.
  • When pasting a resource it might be necessary to 'decapsulate' it, i.e. to move the deeply copied resources as children of the book. AllResourcesPalette, for example, relies on the convention that main resources are children if the book.

• Model changes:
  • DeeplyCopied - Annotation for declaring that a given key should be deeply copied when copying the containing resource.
    • It should be added to ResourceFrame.KEY_MAIN_RESOURCE and StyledElement.KEY_BACKGROUNDIMAGE.
    • Add a method getDeeplyCopiedKeys(Class<R>) in ResourceR4.
      • Add a private boolean deeplyCopied field in ResourceInfoProvider.KeyEntry<R>, so that ResourceR4.getDeeplyCopiedKeys can filter only those entries that have deeplyCopied = true.
  • BaseCompositeElement - Base implementation of CompositeElement. Used to provide a kind for the wrapping fake parent so that it can be correctly persisted.

• Logic for copy/paste of resources:
  • Add three new operations in EditMenuLogic:
    • ON_CUT - handles a user request to cut the current selection.
    • ON_COPY - handles a user request to copy the current selection.
    • ON_PASTE - handles a user request to paste the clipboard contents in the current edit scope.
    • What they do is basically described in the main ideas section.
  • Add a util class (CopyUtil) that contains the following static helper methods user for copy-paste of resources:
    • ResourceModel capsulate(ResourceH resource)
    • void attachModel(ResourceModel childModel, ResourceRefR4 childRef, ResourceChanger changer) - attaches the given ResourceModel to the model of the given changer (at the specified by the given ref location).

• View:
  • Add the following new AppMenuItems in Edit menu:
    • CutItem
    • CopyItem
    • PaseItem
  • Make sure they have the appropriate accelerators (keyboard shortcuts).
    • For Windows, these are Ctrl+X, Ctrl+C and Ctrl+V, respectively.
  • It will be nice, if the Cut and Copy items are enabled only if we have at least one frame or group selected (otherwise pressing them won't do anything). Similarly, the Paste item should be enabled only if the clipboard contains a resource we can paste.
    • However, I doubt whether I will be able to implement this now.

• Dnd related:
  • The existing framework will be very easily used, so no changes are necessary.
  • I added only CommonResourceDataProvider - an extension for the DndDatas that all resources provide:
    • ResourceRefData
    • ResourceRevisionData
    • Maybe I should remove these two datas from the other ResourceDataProviders.

[deni]

• Source code: [8285] and [8289].
• Please, do not review CopyUtilTest , it is for personal testing purposes only, but I committed it by mistake.

[pap]

• I am passing the design but as you can see there are lots of remarks.

• Main ideas - OK and as we talked "decapsulation" is somewhat important(or at least to keep consistency)
• Model changes
  • BaseCompositeElement - to me it doesn't seem like a base class. You don't use it in some othe place but the Copy/Paste. It wold better be named after its real purpose and it may be put in the module where CompositeElement DND/CpPaste is implemented.
  • Why do you make FrameR4 a CompositeElement ?
  • I somewhat dislike the name of the annotation but I cannot think of a really good name.
• Logic for copy/paste and View
  • I am glad that you use the OSUtil for the accelarators
  • I would suggest not using menu items but adding entries to the interacion map of ElementView or BookView that fire copy/cut/paste events. The reason is that with menus we will have inconsistency with already existing copy/paste operations. For example when you have selected some text in a frame Edit->Copy will copy the frame while Ctrl+C will copy the text selection. Otherwise you should fix all other copy/paste interacions which I consider a bad idea.
  • Also I suppose that it is better to put this logic in org.sophie2.main.dnd module or some other place..(not sure). But Edit->Copy as I said may be handled in several different places so it is bad to put this logic here. IMHO this module should just provide the menus.
  • I am not sure that we need this CopyUtil( I may be wrong) but my observations show that sophie has far too many utils and these are not used. So instead of reducing code duplication they help for having such. Also if this is to be used in a single class/enum there is no logic of putting it in a separate place. These methods could just be put in the corresponing logic (CopyFramesLogic or something like that).
• DND related
  • I can't fully get the idea of that CommonResourceDataProvider. But that may be a problem of mine.
• General and code related
  • You have some problems with resource ids that weren't mentioned at all in the design. And these are reallly important if someone should modify your code it is better to have the ideas written in trac than making people search them in the code.
  • I hope you'll get the issues you told me about resolved for the implementation review.
  • I don't see any tests but the one you told me not to look at. I just don't get it. Changes to the model should be verified by tests. Tests written before the code !!! I really can't understand how can you be so confident that you are writing everything correctly. Tests are not a guarantee but at least verify some cases.
  • About the Z-order preserving - you should iterate over all elements in the current scope and check which are selected and use them in that order.
  • About the " COPIED FROM AppHaloUtil:" - first you should write a task comment (TODO:, FIXME: or XXX:) and second I told you how to fix that by just making the AppHaloUtil methods call those of AppViewUtil. You would've done that for almost the same time as wrtitin the comment that you have copied the methods. I just don't get that way of working/thinking - just for your task.
  • Mira gave me some notes about the contents of the CopyUtil and I add some mine
    • First "value instanceof ResourceRefR4" - Key has a method getValueClass that may save you from getting the value which will save time.
    • The model variable in the capsulate method - it isn't necessary to call cloneHeadRevision.
    • Mira says that

" ResourceRefR4 parentRef = valueRef.getParentRef();

ResourceAccess parentAccess = resource.getAccess().open(parentRef, null);

" is a bad practice. Parents should be accessed via views.

[deni]

• ResourceRefs of the copied resources:
  • When pasting resources, I tried to keep the original child references as long as it is possible.
    • This means that if we copy 'FrameABCD' from one page to another, its reference from the book will be './NewPage/FrameABCD'.
    • This ensures that if we copy 'Frame A' and 'Frame B' from one page to another and 'Frame A' had a link rule 'On mouse pressed - Toggle Frame B', this link will be valid in the second page, too.
  • If it is not possible to keep the child ref (for example, when pasting a frame in the same page it originally was), we generate a new ref.
    • The attachModel(...) and capsulate(...) methods in CopyUtil now take a boolean argument that determines what should be done if we try to attach a model to a location which already contains another resource. If it is true, a random ref is generated and the model is attached there; otherwise the existing resource is replaced by the given model.
    • They both return a ResourceRefR4 to the real location where the model was attached.

• Removed the menu items and added interaction map entries for cut, copy and paste in ElementView.
  • Removed the existing entries from FrameView.
  • Moved the ON_COPY and ON_PASTE operations from EditMenuLogic to ElementLogic and CopyUtil to org.sophie2.main.app.commons.element package.
    • I could have moved its methods in ElementLogic, but I felt this class would become too large. If you don't think so, feel free to move them.
  • The was an issue about the scene losing focus when going to another page. This caused Ctrl+V not to work. I added the following fragment to BookView.goToPage(ResourceRefR4):

    if (pwa != null) {
        pwa.getSel().setEditScope(this.getCurrentPageView().mainPartView().get());
    }
    

  • Cutting or pasting a text frame is currently very difficult, because usually its text view is on focus and it has the same interaction entries. It is possible to make the border very large and select the frame itselt by clicking on the border (outside the resize areas).

• Cut functionality:
  • The ON_CUT operation logic is the same as ON_COPY.
  • I couln't think of how to implement ResourceTransferable.deleteOriginal() in general and I decided to override it for the transferable that is created when handling a cut or copy event.
    • I just used ElementH.removeSubElement(ResourceRefR4, String, boolean) .

• Design review related things:
  • FrameR4 will not implement CompositeElement for now. The idea was that if frame resource has a deeply copied key pointing to another frame, for example, the second frame will be copied as a child of the first one and its activation channel will be copied in a frame's KEY_SUB_ELEMENTS.
  • Renamed BaseCompositeElement to CompositeDndElement and moved it to org.sophie2.main.dnd package.
  • The idea of CommonResourceDataProvider was to provide common DndDatas for all resources. All or at least most of the other ResourceDataProviders should extend it, but I just don't more time to do it now. It should not be removed because there is no other ResourceDataProvider for group resources.
  • I left cloneHeadRevision in CopyUtil.capsulate(ResourceH) because I could get a relative reference from the book to the given resource inside capsulate.

• Problems with the current implementation:
  • An exception is thrown when trying to paste a tail text frame.
  • The titles of the copied resources are not changed.
  • The locations of the copied resources are not changed as suggested in the analysis.
  • When copying multiple resources, their relative z-order is not preserved.
  • A few significant AutoActions are registered when cutting and then pasting resources, so undo does not work as expected.
  • When copying a templated frame in another book or instance of Sophie, the templated values are lost.
  • It is difficult to cut/sopy a text frame (instead of the text it contains). Even when you have selected multiple frames one of which is text frame or a group which contains a text frame, sometimes the text view will handle the event.

• Source code: [8285], [8289], [8298], [8300], [8301] and [8302]

[pap]

• Commited in [8355]

[meddle]

Closing all the tickets before M Y1