thieman / github-selfies

Everything is better with selfies. Yes, even pull requests. Trust me.
MIT License
1.14k stars 76 forks source link

Changed the method of linking back to the github repo #77

Closed itsrainy closed 8 years ago

itsrainy commented 8 years ago

This will make it so the images show up in Slack and there is still a link back to your repo. It will look like this:

selfie-0 Github Selfies

thieman commented 8 years ago

Sorry for taking so long to respond to this one, I've been on vacation. Will take it for a spin in the next day or two, thanks!

bhollis commented 8 years ago

Any idea why Slack can't handle an image in a link? I swear it can do it for other sites...

itsrainy commented 8 years ago

I tried a version with the following markdown (spaces on either side of the URLs):

[ ![ selfie-0 ]( http://i.imgur.com/Qc3i2j2.png )]( https://github.com/thieman/github-selfies/ )

It worked as far as displaying the picture goes, but the link did not work. You'll see below there is a stray %7C at the end of the URL, which is a URL encoded pipe character (|). I'm not sure where this is coming from.

image

itsrainy commented 8 years ago

@thieman bump

thieman commented 8 years ago

Ahhhh crap, ok ok looking

thieman commented 8 years ago

Debugging a Race Condition, selfie on HTML5 canvas, 2016

selfie-1 Github Selfies

thieman commented 8 years ago

In terms of actual comments, I'd like to request two changes before we merge this:

  1. Please capitalize the H in GitHub
  2. Ideally, we would only write the linkback once, even if the user takes multiple selfies.

@bhollis Any objections on aesthetic grounds to merging this? Obviously having the linkback as text isn't as pleasing in the GitHub UI, but I think having the selfies show up in Slack is enough of the fun for most people that the tradeoff is probably worth it.

itsrainy commented 8 years ago

I made the changes you described. Although I'm now realizing this makes the linkback display after the first picture and before subsequent pictures, which might be a little strange

thieman commented 8 years ago

Could always add the linkback, but before doing so rewrite any existing linkbacks to empty string?

itsrainy commented 8 years ago

Yea I like that idea. I went ahead and made that change. Let me know if it looks alright to you.

thieman commented 8 years ago

LGTM gonna give Ben a chance to get back

thieman commented 8 years ago

thieman commented 8 years ago

Merged and new versions pushed to the stores, sorry this took an eternity to get done. Life, right?

bhollis commented 8 years ago

It's fine. I'm still amazed that Slack can't deal with a linked image.

On Monday, May 9, 2016, Travis Thieman notifications@github.com wrote:

Merged and new versions pushed to the stores, sorry this took an eternity to get done. Life, right?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/thieman/github-selfies/pull/77#issuecomment-217864008