-
-
Notifications
You must be signed in to change notification settings - Fork 614
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 Plugin: Paste link into editor and it'll be replaced by iframe, using noembed API. #711
Conversation
Seems really cool! :) Remove the min file, it is generated in master branch only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not use anything like babel, so we cannot use let
And you can factor two lines into one.
trumbowyg.execCmd('insertHTML', content); | ||
} | ||
else { | ||
let content = "<a href='"+pastedData+"'>"+pastedData+"</a>"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
content = $('<a>', {
href: pastedData,
text: pastedData
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't let
standard ES6?
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep but Trumbowyg is ES5 for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this:
content = $('<a>', {
href: pastedData,
text: pastedData
})
How do I get a string out of it like content = "<a href='"+pastedData+"'>"+pastedData+"</a>";
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You right.
content = $('<a>', {
href: pastedData,
text: pastedData
}).prop('outerHTML')
data: query, | ||
cache: false, | ||
dataType: "json", | ||
success: function(res) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Init content here:
content = '';
dataType: "json", | ||
success: function(res) { | ||
if (res.html) { | ||
let content = res.html; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove let:
content = res.html;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not use anything like babel, so we cannot use let
And you can factor two lines into one.
success: function(res) { | ||
if (res.html) { | ||
let content = res.html; | ||
trumbowyg.execCmd('insertHTML', content); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this line only one time after the if/else blocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How?
trumbowyg.execCmd('insertHTML', content);
works if content = res.html
trumbowyg.execCmd('insertHTML', content);
returns object when content = $('<a>', {href: pastedData, text: pastedData});
trumbowyg.execCmd('insertHTML', content);
is also needed in the second error. Can't put it outside of the request because that'll make it lose scope.
} | ||
}, | ||
error: function() { | ||
console.log('Error connecting to noembed'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should do the same as the else above instead of nothing.
Looks good to me! :) Can you also add some docs and demo? https://github.com/Alex-D/Trumbowyg/blob/develop/docs/documentation/plugins/index.html Thanks :) |
… the future. Spotify is now embeddable ;-)
fails++; | ||
}, | ||
complete: function() { | ||
console.log(this.url); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove console log please :)
complete: function() { | ||
console.log(this.url); | ||
if (fails === 1) { | ||
console.log(fails); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log
$.ajax(this); | ||
} | ||
if (fails === 2) { | ||
console.log(fails); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log
…represents more what it does. Removed log outputs;
|
||
request = $.ajax({ | ||
crossOrigin: true, | ||
url: "https://noembed.com/embed?nowrap=on", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about having those urls configurable?
Would be great, so someone can use his own service to handle pasted urls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some options like noembed plugin do?
|
||
$.extend(true, $.trumbowyg, { | ||
plugins: { | ||
pasteImage: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oups. I think this should be "pasteEmbed"
Maybe @Alex-D you can merge and make the requested changes? /* ===========================================================
* trumbowyg.pasteembed.js v1.0
* Url paste to iframe with noembed. Plugin for Trumbowyg
* http://alex-d.github.com/Trumbowyg
* ===========================================================
* Author : Max Seelig
* Facebook : https://facebook.com/maxse
* Website : https://www.maxmade.nl/
*/
(function($) {
"use strict";
var defaultOptions = {
enabled: true,
endpoints: [
"https://noembed.com/embed?nowrap=on",
"https://api.maxmade.nl/url2iframe/embed"
]
};
$.extend(true, $.trumbowyg, {
plugins: {
pasteEmbed: {
init: function(trumbowyg) {
trumbowyg.o.plugins.pasteEmbed = $.extend(true, {}, defaultOptions, trumbowyg.o.plugins.pasteEmbed || {});
if (!trumbowyg.o.plugins.pasteEmbed.enabled) {
return;
}
trumbowyg.pasteHandlers.push(function(pasteEvent) {
try {
var clipboardData = (pasteEvent.originalEvent || pasteEvent).clipboardData,
pastedData = clipboardData.getData("Text"),
endpoints = trumbowyg.o.plugins.pasteEmbed.endpoints,
request = null;
if (pastedData.startsWith("http")) {
pasteEvent.stopPropagation();
pasteEvent.preventDefault();
var query = {
url: pastedData.trim()
};
var content = "";
var index = 0;
if (request && request.transport) request.transport.abort();
request = $.ajax({
crossOrigin: true,
url: endpoints[index],
type: "GET",
data: query,
cache: false,
dataType: "jsonp",
success: function(res) {
if (res.html) {
index = 0;
content = res.html;
} else {
index++;
}
},
error: function(res) {
index++;
},
complete: function() {
if (content.length == 0 && index < endpoints.length - 1) {
this.url = endpoints[index];
this.data = query;
$.ajax(this);
}
if (index == endpoints.length - 1) {
content = $("<a>", {
href: pastedData,
text: pastedData
}).prop('outerHTML');
}
if (content.length > 0) {
index = 0;
trumbowyg.execCmd("insertHTML", content);
}
}
});
}
} catch (c) {}
});
}
}
}
});
})(jQuery); |
Thanks! :) |
Users don't know noembed, embedding should happen automatically. That's what this plugin is for. It looks for paste event, checks if it's a url and uses noembed API to retrieve an iframe from that url. If it can't retrieve an iframe, it will put an anchor tag around the url.