Opened 12 years ago

Closed 11 years ago

Last modified 11 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:


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?

Attachments (1)

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

Download all attachments as: .zip

Change History (18)

comment:1 by Aleksej, 12 years ago

Cc: deletesoftware@… added

comment:2 by Emily O'Leary, 12 years ago

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 by Christopher Allan Webber, 11 years ago

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

comment:4 by Emily O'Leary, 11 years ago

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 by Christopher Allan Webber, 11 years ago

Any updates on this?

comment:6 by Stephen Compall, 11 years ago

Cc: Stephen Compall added

comment:7 by Emily O'Leary, 11 years ago

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.

comment:8 by Emily O'Leary, 11 years ago

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

comment:9 by Emily O'Leary, 11 years ago

Cc: Emily O'Leary added

comment:11 by Christopher Allan Webber, 11 years ago

Keywords: review added

comment:12 by Christopher Allan Webber, 11 years ago

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 by Emily O'Leary, 11 years ago

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 by Christopher Allan Webber, 11 years ago

Keywords: review removed
Status: assignedin_progress

comment:15 by Emily O'Leary, 11 years ago

Owner: Emily O'Leary removed
Status: in_progressreview

I've made all those changes here->

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

by Emily O'Leary, 11 years ago

An added patch for convenience.

comment:16 by rodney757, 11 years ago

Resolution: fixed
Status: reviewclosed

Great! Merged. Thank you.

comment:17 by rodney757, 11 years ago

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