Ticket #2351 (closed bug: obsolete)

Opened 10 years ago

Last modified 9 years ago

sophiex-dnd-between-two-books - Try to drag and drop resource from one book to another throws an exception.

Reported by: todor Owned by: deni
Priority: major Milestone: X3
Component: uncategorized Version: 2.0
Keywords: Cc:
Category: unknown Effort:
Importance: Ticket_group:
Estimated Number of Hours: 0 Add Hours to Ticket: 0
Billable?: yes Total Hours: 0
Analysis_owners: todor Design_owners: deni
Imp._owners: deni Test_owners:
Analysis_reviewers: deni Changelog: Changelog
Design_reviewers: meddle Imp._reviewers: meddle, todor, deyan
Test_reviewers: Analysis_score: 3.5
Design_score: 3.5 Imp._score: 4
Test_score: 0

Description (last modified by todor) (diff)

  1. Open 2 books.
  2. Insert frames in each book.
  3. Try to drag and drop resource form the first book's resource palette to the frame in the second book (as content (the one)).
  4. Exception is thrown instead of importing the resource and applying it where necessary (as background content, etc.)
    • (optional) If this is possible drag and drop between two applications, should behave as it would in one.
      • If you dnd resource it should be imported first and then used as the user wants.
      • If a template is dnd from one app to another first it should be added as template there, and then applied over the drop zone.

Attachments

dnd-between-two-books.patch (15.1 KB) - added by deni 10 years ago.

Change History

comment:1 Changed 10 years ago by deyan

  • Description modified (diff)

Batch update from file active_tickets.csv

comment:2 Changed 10 years ago by todor

  • Status changed from new to s1b_analysis_finished
  • Description modified (diff)

comment:3 Changed 10 years ago by deni

  • Design_owners set to deni
  • Status changed from s1b_analysis_finished to s2a_design_started
  • Imp._owners set to deni
  • Analysis_reviewers set to deni
  • Analysis_score changed from 0 to 3.5
  • I somewhat dislike the structure of the ticket, but the problem and the expected behaviour can be understood.

Changed 10 years ago by deni

comment:4 Changed 10 years ago by deni

  • Status changed from s2a_design_started to s2b_design_finished
  • Changing the main resource of a frame:
    • Override the getResourceData(DndTransferable) method in SophieFormatImportManager to look for ResourceRevisionData in the transferable and if possible - to use it.
    • Add a new method in ResourceImportUtil - public static ResourceRefR4 importResource(BookH, DndTransferable) that imports the Sophie2 resource transferred by the given DndTransferable as a child of the given book. It returns a ResourceRefR4 pointing to the newly imported resource.
    • In ChangeFrameMainResourceHandler.handleDrop(DndTransferable) do not use the ResourceRefData if the resource ref is not inner for the book. Instead, import the resource using the new method and use reference it returned.
  • Changing frame background:
    • Should be similar to changing the main resource.
    • The changes will be in ChangeBackgroundHandler.
  • Applying a template from one book to a resource from another:
    • Again we should not use TemplateRefData if the reference is not inner for the book in which the template wil be applied.
    • Instead we should use a new data - TemplateRevisionData which will transfer revisions of template resources. It will be analogous to TemplateRefData and will be added in TemplateTRansferable.
    • No changes to the export and pre-import logics will be necessary because the book from which the templates are exported is the current book (otherwise it would be more difficult to check whether a template is a page or frame template, for example).
    • The changes should be in ApplyFrameTemplateHandler, ApplyPageTemplateHandler, CreateFrameByTemplateHandler and maybe in CreatePageByTemplateHandler (althought currently our user interactions do not allow creating a page from template from another book).
    • Since main.apps.layout module does not have a dependency from main.func.resources, templating functionality will be extracted in a new module.

comment:5 Changed 10 years ago by meddle

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_reviewers set to meddle
  • Ok I think it passes, but thin new method in the ResourceImportUtil can be avoided, you can use the dropResource method passing the accurate provider and making checks there. I think that way is more clear.
  • And please give names and descriptions of any new modules you will add.
  • You can synchronize with pap and me for namings and import logic/utilities changes, if something is not possible with the current state, there is no problem adding new methods/classes, but otherwise I think it is bad.

3.5p

comment:6 Changed 10 years ago by deni

  • Owner set to deni
  • Status changed from s2c_design_ok to s3a_implementation_started

comment:7 Changed 10 years ago by deni

  • Status changed from s3a_implementation_started to s3b_implementation_finished

Templates module

  • It is called org.sophie2.main.func.templates and provides template functionality.
  • The templates module depends on:
    • main.func.resources
  • The following modules depend on the templates module:
    • dev
    • extra.func.embedded
  • The templates module contains the following packages:
    • package org.sophie2.main.func.templates
      • MainFuncTemplatesModule - module class for template functionality
      • TemplatePaletteUtil - utility class that contains helper methods used to reduce code duplication in the template palettes.
    • package org.sophie2.main.func.templates.book
    • package org.sophie2.main.func.templates.page
    • package org.sophie2.main.func.templates.frame
  • Moved to the new module all classes related to templates from org.sophie2.main.app.layout.right.library package and SaveBookAsTemplateItem
  • Added 3 new logics for events related to book, page and frame templates, respectively.
    • Moved the operations from LibraryTabLogic there.
    • Moved the operations for applying templates via DnD over page preview and page structure palette in PageTemplatesLogic.
  • Moved EmbedBookTemplateHandler in extra.func.embedded module and the operation for embedding books by template in EmbeddedBookLogic.
    • It is still not working... and I can't fix it now.

Applying a template from one book to an element in another

  • Will not be implemented now, because the behavior is not clear and the implementation will not be easy. If a new ticket is created, the following cases should be considered:
    • a template extends another template
    • a frame template includes the main resource/background image
  • In order to disable applying a template from one book to another:
    • in all pre_import logics do not use the current book, but the book in which the template will be dropped. Its book view can usually be found using the source of the event and ElementView.getBookView().

A template's options which keys to include are not persisted

  • Steps to reproduce:
    • Create a frame template, which does not include the main resource
    • Save the book
    • Open the book
    • Apply the template to see that it includes the main resource.
  • Reason: TemplatedKey has a SimpleKey that indicates whether the key should be set to USE_TEMPLATE when a template is applied. This simple key is not persisted.
  • Fix (thanks to meddle):
    • Add a new method in Key - getSubKeys() which by default returns an empty List.
    • Override it in TemplatedKey to return KEY_APPLY and KEY_LOCKED.
    • In ResourceInfoProvider.getKeysByReflection() add to the result not only the keys, but also their sub-keys.

The "Add as template" dialog's X button does not close it as cancel

  • This happens only if we have created a template and not pressed 'Cancel' since then. The reason is that TemplateDialog.SwingDialog keeps the last TemplateInfo result and uses it. Pressing 'Cancel' set is to null while pressing the close button does not.
  • TemplateDialog.show(Input) after showing the dialog and getting the result, set the result info to null.


comment:8 Changed 10 years ago by deni

  • Design_score changed from 0 to 3.5
  • Source code: [8845], [8886] and [8887]
  • Meddle, you didn't write my design score in the field!!!

comment:9 Changed 10 years ago by meddle

Hehe I guess your influence over me is huge :P

comment:10 Changed 10 years ago by deni

  • Removed the fix for persisting TemplatedKey's KEY_APPLY and KEY_LOCKED, because it caused another problem - when dragging the head frame of an autochain to the work area, an exception was thrown.
  • In SophieDragDropHandler.drop(DropTargetDropEvent) surrounded callng DropHandler.handleDrop(DndTransferable) with try block and moved changing the current handler in the finally block. In this way, the visual indications will be removed, even if there is an exception in the drop handler.
  • Source code: [8896]

comment:11 Changed 10 years ago by meddle

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 0 to 4
  • Imp._reviewers set to meddle, todor, deyan

Merged to the trunk at [8898].

  • I think that your description of the design changes is fabulous!
  • I take the new module as a code improvement.
  • Of course I don't like the new method in the ResourceImportUtil, move the DND methods to another util soon, please!
  • Sorry for the delay of the review.

comment:12 Changed 9 years ago by meddle

  • Status changed from s3c_implementation_ok to closed
  • Resolution set to obsolete

Closing all the tickets before M Y1

Note: See TracTickets for help on using tickets.