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

Customizable options: originalContent and changedContent #9

Open
hlridge opened this issue Jan 12, 2015 · 4 comments
Open

Customizable options: originalContent and changedContent #9

hlridge opened this issue Jan 12, 2015 · 4 comments

Comments

@hlridge
Copy link

hlridge commented Jan 12, 2015

Setting originalContent and changedContent didn't work for me. I changed these two lines:

original = $(settings.originalContainer, this).text();
changed = $(settings.changedContainer, this).text();

... to

original = settings.originalContent || $(settings.originalContainer, this).text();
changed = settings.changedContent || $(settings.changedContainer, this).text();

... to use originalContent and changedContent when set, rather than originalContainer and changedContainer.

@arnab
Copy link
Owner

arnab commented Jan 12, 2015

Hello @hl222ih - thanks for reporting this. I should really add some tests to this. It started as a quick get-it-done wrapper. For now, can you share a jsfiddle showing the problem? To me, looking at the code, it seems that, if you set the options like this:

$(selector).prettyTextDiff({
  originalContent: 'foo',
  changedContent: 'foobar'
});

... it should work, since the settings are extended from the given ones.

The jsfiddle demo added to the README also shows such an example.

@hlridge
Copy link
Author

hlridge commented Jan 14, 2015

Hi!

What I can see from your jsfiddle demo you are referencing v 1.0.4
(https://rawgit.com/shikher/jQuery.PrettyTextDiff/01222bca2a130133168dd43c9c289322f7ed9e20/jquery.pretty-text-diff.js)
which indeed has code for originalContent/changedContent, but in your
github repo only v1.0.3 is shown which does not have any code for
originalContent and changedContent. (Though the coffeescript seems
updated). Also in http://plugins.jquery.com/pretty-text-diff/ only
version up to 1.0.3 is hosted. And it was with v1.0.3 I was experiencing
this problem. So probably it was just because of this. :)
With your demo with v1.0.3. http://jsfiddle.net/q7hyfev8/124/ it is
shown it is shown that nothing happens when clicking [Diff-Functional
Parameter].

kind regards,
Hannes Ljusås

------ Originalmeddelande ------
Från: "Arnab Deka" [email protected]
Till: "arnab/jQuery.PrettyTextDiff"
[email protected]
Kopia: "Hannes" [email protected]
Skickat: 2015-01-12 17:25:00
Ämne: Re: [jQuery.PrettyTextDiff] Customizable options: originalContent
and changedContent (#9)

Hello @hl222ih - thanks for reporting this. I should really add some
tests to this. It started as a quick get-it-done wrapper. For now, can
you share a jsfiddle showing the problem? To me, looking at the code,
it seems that, if you set the options like this:

$(selector).prettyTextDiff({ 'originalContent': 'foo',
'changedContent': 'foobar' });
... it should work, since the settings are extended from the given
ones.

The jsfiddle demo added to the README also shows such an example.


Reply to this email directly or view it on GitHub.

@fjahr
Copy link

fjahr commented Apr 29, 2015

@hl222ih is right, I also just stumbled across that problem as well. 1.0.4 does what the readme says, 1.0.3 does not. You must have built 1.0.4 in coffee but then not compiled it to JS and pushed it to the repository, as the JS versions in the repo are still on 1.0.3. I think you should also replace the jquery plugin because your readme is the only real source for documentation and it is confusing that the latest jquery plugin version does not work like the docs indicate.

Best,
Fabian

arnab added a commit that referenced this issue Apr 29, 2015
@arnab
Copy link
Owner

arnab commented Apr 29, 2015

Ah that sounds about right. Thanks @fjahr and @hl222ih - this must have been because I merged in PR #6 but forgot to build/push the JS. Thanks for reporting it.

It's all fixed now. Also, jQueryPlugins site appears deprecated and suggests to move to npm.js - I published this package there (iunfortunately jQuery-plugins-site is read only and so there is no way to say it there).

Also, added a v.1.0.4+ subscript to these new options so it's less puzzling when it doesn't work.

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

No branches or pull requests

3 participants