innoq / statuses

statuses
Apache License 2.0
13 stars 14 forks source link

Add image preview #70 #71

Closed aheusingfeld closed 9 years ago

aheusingfeld commented 9 years ago

@FND though you probably don't like #70, could you review the JS? Thanks :)

The idea of this is to add JavaScript code to the page which iterates over all posts and check whether there is text in the post that matches a certain pattern. This pattern would match 1.) A URL prefixed with ! which is present anywhere in the post 2.) A valid markdown image syntax like ![alternative text](http://my.url/great-image.gif) If a matching text is found, the code will append an <img> tag to the post to render the (potential) image

FND commented 9 years ago

I'll do some refactoring on the train tonight.

stilkov commented 9 years ago

Let me know when you’re done and I’ll merge.

aheusingfeld commented 9 years ago

I'll do some refactoring on the train tonight.

Refactoring to leverage HEAD requests in the backend? I think my Clojure knowhow is a bit to rusty to implement a quick solution for this :|

stilkov commented 9 years ago

What HEAD request are you talking about?

aheusingfeld commented 9 years ago

@stilkov https://github.com/innoq/statuses/issues/70#issuecomment-60189389

aheusingfeld commented 9 years ago

@FND I adopted your review in https://github.com/aheusingfeld/statuses/commit/4aed8a093a37b4b2e83e38d0721de116a680a9d5. Thanks again!

aheusingfeld commented 9 years ago

More descriptive than https://github.com/aheusingfeld/statuses/commit/6c955e25baa5e8f859544019f7963c00ef62c812? :)

FND commented 9 years ago

don't remove the image tag from the original post

That just tells me what was changed, but doesn't provide any rationale or context. But I guess that's a different discussion.

aheusingfeld commented 9 years ago

Honestly I'm always happy to evolve, so feedback is very welcome! I added a longer description to this pull-request now.

FND commented 9 years ago

That's very useful, thanks (actually clarifies one of your comments elsewhere).

However, I would have liked to see that in the commit itself (repo history is more immediately accessible and less ephemeral) - so if you're gonna squash anyway, might as well amend the commit message accordingly.

FND commented 9 years ago

When you squash those commits, better create a new PR; this one's now fairly chaotic.