Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added “Paste and Keep Data” feature to draw tools plugin #914

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

markfguerra
Copy link
Contributor

When I'm exchanging draw tools data with others, I sometimes want to combine my drawing with theirs to make one unified drawing. This change allows users to do exactly that.

This change adds an additional button to the "Drawtools Opt" options, labeled "Paste and Keep Data". It behaves the same way as "Paste Drawn Items", except that it also merges the existing draw data with the pasted data using the jQuery method $.merge().

@tapionx
Copy link

tapionx commented Dec 10, 2014

It would be better to call the button 'Add drawings'

@markfguerra
Copy link
Contributor Author

"Add drawings" is indeed a better label. I'll push an update later.

@markfguerra
Copy link
Contributor Author

I realized after pushing the code that $.merge() will allow duplicate draw objects if such a duplicate exists in both the existing and newly pasted data.

I'm working on a solution that merges the drawings while also removing duplicates.

@PrinterElf
Copy link

I suggest that 'Paste (Add Drawn Items)' would be a better name - 'Add Drawings' could be ambiguous as to the source.
The existing button would need renaming to 'Paste (Replace Drawn Items)'.

@jonatkins
Copy link
Collaborator

Suggestion: rather than keeping the existing paste entry method, that uses a standard javascript "prompt" function for text entry, how about a dialog containing a form, with a text field to paste into, and "import", "merge" and "cancel" buttons?

@tapionx
Copy link

tapionx commented Dec 10, 2014

@johnatkins +42
This would be the most clear option.

@markfguerra
Copy link
Contributor Author

To resolve the duplicates on merge, I wrote a working solution that utilizes mergeDeep(), which I got from here:

https://gist.github.com/sinemetu1/1732896
http://samonstuff.blogspot.com/2012/02/merging-json-objects.html

I reached out to the developer to see if this solution has a license and whether it's usable for Open Source. It's not explicitly stated on his site as far as I can see. Although I tried several times, I have not heard back from him.

@markfguerra
Copy link
Contributor Author

I think it's safe to say I won't be hearing back from the mergeDeep() author. Do you folks think his method is still usable?

@pleasantone
Copy link
Contributor

It's a fairly simple and obvious algorithm, my inclination is to allow it to inspire you and credit the author.

@markfguerra
Copy link
Contributor Author

@pleasantone Makes sense. Free time is always the issue

@pleasantone
Copy link
Contributor

This is also similar to #1033 which already handles the merges, and also may have the benefit of opening properly on an actual passed url. i.e. click a stock link.

@davidlinse
Copy link

any news on this ?
regards
~david

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants