Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#492 closed enhancement (fixed)

comment preview

Reported by: Will Kahn-Greene Owned by:
Priority: major Milestone: 0.5.0
Component: programming Keywords:
Cc: deletesoftware@…, Stephen Compall, Emily O'Leary Parent Tickets:

Description

Given that comments use a markup, it'd help a lot to have comment preview.

I've seen this work in a few ways on the internet.

Trac shows you the preview while you're writing the comment in a different box. I like that--it's pretty helpful.

MediaWiki has a "preview" button and shows you what it'll look like when it's posted. Pyblosxom works this way, too. This works, but it's far less convenient.

I'd prefer us to implement the first. Maybe use AJAX to round-trip formatting the comment to get the preview every few seconds? Is that safe?

Subtickets

Attachments (1)

trac_492_comment_preview.patch (12.6 KB) - added by Emily O'Leary 9 years ago.
An added patch for convenience.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 10 years ago by Aleksej

Cc: deletesoftware@… added

comment:2 Changed 10 years ago by Emily O'Leary

Owner: set to Emily O'Leary
Status: newaccepted

I like the "as you edit" automatic preview option more than the preview button.

I think that a simple bit of AJAX could probably do that. Alternatively you could have the Javascript parse the markup instead and update the preview box without pinging the server for it.

comment:3 Changed 10 years ago by Christopher Allan Webber

As a minor update, I just spoke to LotusEcho on IM yesterday, and I know she's working on this.

comment:4 Changed 10 years ago by Emily O'Leary

I've set up a lot of the static client side stuff (div, modified Show Comment Javascript). Now I'm just working through setting up the back end view and making it return the formatted text.

comment:5 Changed 10 years ago by Christopher Allan Webber

Any updates on this?

comment:6 Changed 10 years ago by Stephen Compall

Cc: Stephen Compall added

comment:7 Changed 10 years ago by Emily O'Leary

I've implemented this. It shows a comment preview in real-time as you type with 500ms delay. The following commit in my feature branch explains it better.

https://gitorious.org/~lotusecho/mediagoblin/lotusechos-mediagoblin/commit/fd215812087d7e3ceb8c31d754a5cda7bb7e8e9b

comment:8 Changed 10 years ago by Emily O'Leary

Owner: changed from Emily O'Leary to Christopher Allan Webber
Status: acceptedassigned

comment:9 Changed 10 years ago by Emily O'Leary

Cc: Emily O'Leary added

comment:11 Changed 10 years ago by Christopher Allan Webber

Keywords: review added

comment:12 Changed 10 years ago by Christopher Allan Webber

Heya LotusEcho,

You've done some really great work on this; I tested it and it works really nicely. Thanks for it!

I think there are some issues that need to be addressed before we can pull it in.

  • These commit lines are really, really long. This makes it fairly hard to read the logs; it's best if you wrap at 80 characters.
  • I see print statements and commented out statements in the patch:
+    print request.form['comment_content']

+    print comment
+    #decoderRing = json.JSONDecoder()
+   #comment = decoderRing.decode(request.query_string)

we need this stuff cleaned up before it can be merged.

  • Re: this commit:
"Added i18n to my javascript changes as per trac #417"

hm.... is this js file being "intercepted" by mediagoblin? If not, how is it being translated?

I don't think this will be translated in reality since it isn't passing through jinja2.

yep, not translated.

How about this? Just put the "comment preview" bit on the page and hide it by default. Then un-hide it with css once it's ready to be shown.

  • It works really well. I think it's updating too often though... 500ms is really fast. I think we could at least put it at a second. It might be nice if this was configurable. However, the only way I think we could configure this would be to put it in a config setting that we then embed in the page like:
<script>var comment_update_interval = 500</script>
  • Is it checking content against comment_content to see if it changed?
+	if ($('#comment_content').val() && (content != $('#comment_content').val())) {
+		content = $('#comment_content').val();

That makes sense. Maybe it could benefit from a docstring?

  • What's going on here?
    comment = unicode(urllib.unquote(request.query_string).decode('string_escape'))
    if comment.startswith('"') and comment.endswith('"'):
        comment = comment[1:-1]

What is this "string_escape" decoding?

Ah, I see.. So you're both encoding and decoding the out from the request to/from json? However, the request is just a string... maybe json doesn't actually do anything for us. Maybe we should just skip the json encoding/decoding bit? Alternately, if we keep it, we should switch it to a hashmap with a key for "comment" and then the body there.

Note: the only reason we'd need to do this so far is to supply different rendering systems for comments (restructured text, etc) which is kind of moot because we don't presently support that behavior.

comment:13 Changed 10 years ago by Emily O'Leary

Owner: changed from Christopher Allan Webber to Emily O'Leary

I need to revisit the way it's encoded. There were some oddities as I was working through it at Libre Planet and after some time cooling off I think I can approach it from a different angle and make it work better.

I will try to address these comments today and whip up a new patch. :)

comment:14 Changed 10 years ago by Christopher Allan Webber

Keywords: review removed
Status: assignedin_progress

comment:15 Changed 9 years ago by Emily O'Leary

Owner: Emily O'Leary deleted
Status: in_progressreview

I've made all those changes here-> https://gitorious.org/~lotusecho/mediagoblin/lotusechos-mediagoblin/commits/trac_492_comment_preview

I ended up serializing the entire form (so it would pass the CSRF token) which made everything a whole lot easier.

Changed 9 years ago by Emily O'Leary

An added patch for convenience.

comment:16 Changed 9 years ago by rodney757

Resolution: fixed
Status: reviewclosed

Great! Merged. Thank you.

comment:17 Changed 9 years ago by rodney757

Milestone: 0.5.0
Note: See TracTickets for help on using tickets.