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.

Sign up for GitHub

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

Jump to bottom

Fixing link embedding sometimes messing up, fixed vimeo embed not matchi... #343

Merged
merged 1 commit into from
Apr 19, 2014
Merged

Fixing link embedding sometimes messing up, fixed vimeo embed not matchi... #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/assets/javascripts/backbone/plugins/image_embed.js.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class Kandan.Plugins.ImageEmbed
@options:
regex: /http.*\.(jpg|jpeg|gif|png)/i
regex: /http[\S]*\.(jpg|jpeg|gif|png)/i
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use a dot, then things like http://static.azgor.com/wp-content/uploads/octobiwan.jpg http://static.azgor.com/wp-content/uploads/octobiwan.jpg end up matching, even though they are two separate links. We should work off of non-space characters, vs all characters.


template: _.template '''
<div class="image-preview">
Expand All @@ -15,8 +15,10 @@ class Kandan.Plugins.ImageEmbed
@init: ()->
Kandan.Modifiers.register @options.regex, (message, activity) =>
url = message.match(@options.regex)[0]
startIndex = message.match(@options.regex).index
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this might be a little controversial, but for cases like http://static.azgor.com/wp-content/uploads/octobiwan.jpg http://static.azgor.com/wp-content/uploads/octobiwan.jpg, I feel like we should only match the first case of the image, and anything else is translated into the subtitle. So like http://static.azgor.com/wp-content/uploads/octobiwan.jpg http://static.azgor.com/wp-content/uploads/octobiwan.jpg here I am, it would be an image with the subtitle http://static.azgor.com/wp-content/uploads/octobiwan.jpg here I am

I think this makes more sense than having each image in a message imageified, since anything after what you are trying to embed is usually the subtitle rather than multiple embeds.

endIndex = startIndex + url.length
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before http://static.azgor.com/wp-content/uploads/octobiwan.jpg blah cool http://www.facebook.com

Ends up being an image with subtitle before blah cool http://www.facebook.com

fileName = url.split("/").pop()
comment = $.trim(message.split(url).join(""))
comment = $.trim(message.replace(message.substring(startIndex, endIndex),""))
subtitle = null
subtitle = comment if comment.length > 0
subtitle ||= fileName
Expand Down
4 changes: 2 additions & 2 deletions app/assets/javascripts/backbone/plugins/link_embed.js.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
class Kandan.Plugins.LinkEmbed

@options:
regex: /(^| )(http?\S*)/g
regex: /(^| )(http?[^\s<>]*)/g
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes the link embed would pull in the tag after the link, which we don't want. A url should never have a < or > in it, so let's make it a negative class group that wont match spaces or carots.


@init: ()->
Kandan.Modifiers.register @options.regex, (message, activity)=>
message = message.replace(@options.regex, '<a target="_blank" href="$2">$2</a>')
message = message.replace(@options.regex, '$1<a target="_blank" href="$2">$2</a>')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you said "Hey lets check out http://www.facebook.com" it would end up looking like

Hey lets check outhttp://www.facebook.com (note the missing space)

return message
2 changes: 1 addition & 1 deletion app/assets/javascripts/backbone/plugins/vimeo_embed.js.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Kandan.Plugins.VimeoEmbed
videoId = message.match(@options.regex)[2]

subtitle = null
subtitle = "Vimeo: #{comment}" if comment? and comment.length > 0
subtitle = "#{comment}" if comment? and comment.length > 0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Youtube embed doesn't do this, so lets not do this for Vimeo, or at least have it the same across both.

subtitle ||= videoUrl

message = @options.template({
Expand Down