Edgewall Software
Modify

Opened 11 years ago

Closed 11 years ago

#8855 closed enhancement (fixed)

Add automatic ticket comment preview

Reported by: Remy Blank Owned by: Remy Blank
Priority: normal Milestone: 0.12
Component: ticket system Version: devel
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Even though WikiFormatting is quite simple to write, writing nicely-formatted texts requires repeated usage of the "Preview" button. This could be improved by updating the preview automatically when the associated <textarea> is modified.

This ticket proposes adding this functionality to the ticket comment <textarea> and the ticket comment edit <textarea>. Patch follows.

Attachments (2)

8855-ticket-auto-preview-r8861.patch (12.9 KB ) - added by Remy Blank 11 years ago.
Automatic preview for ticket comments and ticket comment edits.
8855-ticket-auto-preview-r8872.patch (19.3 KB ) - added by Remy Blank 11 years ago.
Adds a generic Python → JavaScript data converter.

Download all attachments as: .zip

Change History (18)

by Remy Blank, 11 years ago

Automatic preview for ticket comments and ticket comment edits.

comment:1 by Remy Blank, 11 years ago

The patch above adds an automatic preview to both the ticket comment box and the ticket comment editor. An update is triggered one second after the last change, but only if the text has actually changed. The interval is configurable, and setting it to zero will disable the automatic preview.

The patch adds a generic wiki text previewer componnet WikiPreview. It takes requests on /wiki_preview, and renders a text passed in the text argument using format_to(), with a context given by the realm, id and version arguments. The rendering flavor (html, oneliner) can be set with the flavor argument, and the escape_newlines and shorten arguments are also accepted.

Comments welcome.

comment:2 by Christian Boos, 11 years ago

I like it, and the code is very clean as well.

For in place comment editing, the preview is still above, though.

There's also no automatic preview for the description when creating tickets, but there having them stacked vertically is not optimal. However, a side-by-side mode wouldn't play well with the fixed width we currently have.

Getting rid of that fixed width would be an option…

Now I'll try to see how this can be adapted for the wiki preview (#8721).

in reply to:  2 ; comment:3 by Christian Boos, 11 years ago

Now I'll try to see how this can be adapted for the wiki preview (#8721).

  • trac/wiki/templates/wiki_edit.html

    diff --git a/trac/wiki/templates/wiki_edit.html b/trac/wiki/templates/wiki_edit.html
    a b  
    2929          autoResizeText();
    3030          $(window).resize(autoResizeText);
    3131        </py:if>
     32        <py:if test="sidebyside and not diff">
     33          $("#text").autoPreview(${auto_preview_timeout}, "${href.wiki_preview()}", {
     34              realm: "${page.resource.realm}", id: "${page.resource.id}",
     35              __FORM_TOKEN: "${req.form_token}"
     36            }, function(textarea, text, rendered) {
     37              $("#preview div.wikipage").html(rendered); // always show, even if empty
     38              autoResizeText();
     39          });
     40        </py:if>
    3241      });
    3342    </script>
    3443  </head>
  • trac/wiki/web_ui.py

    diff --git a/trac/wiki/web_ui.py b/trac/wiki/web_ui.py
    a b  
    451451            'edit_rows': editrows, 'sidebyside': sidebyside,
    452452            'scroll_bar_pos': req.args.get('scroll_bar_pos', ''),
    453453            'diff': None,
     454            'auto_preview_timeout': WikiPreview(self.env).auto_preview_timeout,
    454455        })
    455456        if action in ('diff', 'merge'):
    456457            old_text = original_text and original_text.splitlines() or []
     
    466467        self._wiki_ctxtnav(req, page)
    467468        add_script(req, 'common/js/wikitoolbar.js')
    468469        add_script(req, 'common/js/resizer.js')
     470        add_script(req, 'common/js/auto_preview.js')
    469471        return 'wiki_edit.html', data, None
    470472
    471473    def _render_history(self, req, page):

(on top of ticket:8721:auto-resize-js.patch, after rebasing the whole patch queue of that ticket on top of 8855-ticket-auto-preview-r8861.patch)

Works great!

However I expect this will be quite resource intensive, so maybe, in addition of the delay thing, we could think about a mode which would be less "real-time". For example, the update could be triggered only when pressing Enter and on blur. For example, only

in reply to:  3 comment:4 by Remy Blank, 11 years ago

Replying to cboos:

Works great!

Thanks for testing! There's one detail I'm not happy about yet: the name and location of WikiPreview. While that component is currently used for automatic previews, it's actually a generic HTTP wiki formatter. So I suggest we rename the component to WikiFormatter, move it to trac.wiki.formatter, and use the path /wiki_format instead of /wiki_preview for the URL.

Also, is the auto_preview_timeout option correctly placed in [trac] or should I move it to [wiki]?

Opinions? Better ideas?

comment:5 by Remy Blank, 11 years ago

Ok, maybe not move it to trac.wiki.formatter, as trac.wiki.web_ui is probably the right location. But the name and path changes make sense.

comment:6 by Christian Boos, 11 years ago

I'd suggest a separate file, trac.wiki.web_api, so that people having disabled the wiki (shame to them!) via trac.wiki.web_ui.* = disabled could still get automatic wiki preview for tickets.

The component itself could be name WikiRenderer, but I'm fine with WikiFormatter as well.

by Remy Blank, 11 years ago

Adds a generic Python → JavaScript data converter.

comment:7 by Remy Blank, 11 years ago

The patch above is an update with the following changes:

  • Renamed and moved WikiPreview as suggested in comment:6.
  • Moved the ticket comment edit preview below the <textarea>.
  • I didn't like how the auto_preview_timeout and __FORM_TOKEN values had to be passed manually at every call to autoPreview(). They are now passed automatically to auto_preview.js through a generic JavaScript data provider, chrome.add_script_data(). I believe this mechanism is quite useful, can also be used for the field data in query.html as well, for example.

With this last change, adding an automatic preview requires two changes:

  • Add a call to Chrome(self.env).add_auto_preview(req) in the request handler.
  • Add a call to $(...).autoPreview(href, args, update) in the template with args containing only application-relevant data.

comment:8 by Christian Boos, 11 years ago

Wouldn't render_javascript be better placed in source:trunk/trac/util/presentation.py? I think it would be both more efficient and more readable if it was written in python rather than as a template function.

And it could then be renamed to_json ;-)

comment:9 by Remy Blank, 11 years ago

Yes, that makes sense. I was too absorbed in template programming to even think about the alternative. to_json() sounds good, too, and now that you mention JSON, maybe I can even find an existing implementation somewhere…

Ooh, Python 2.6 has a json module that does exactly what we need, so maybe trunk/trac/util/compat.py is an even better location ;-)

in reply to:  9 comment:10 by Christian Boos, 11 years ago

Replying to rblank:

Yes, that makes sense. I was too absorbed in template programming to even think about the alternative. to_json() sounds good, too, and now that you mention JSON, maybe I can even find an existing implementation somewhere…

I'll ask Dirkjan if he can donate us what he did for Mercurial…

Ooh, Python 2.6 has a json module that does exactly what we need, so maybe trunk/trac/util/compat.py is an even better location ;-)

For the equivalent to json.dumps, but it would still be more convenient to have a little to_json helper function in presentation.

comment:11 by Remy Blank, 11 years ago

Functionality committed in [8884] and set the default delay to 2 seconds in [8885]. The to_json() function uses the json module if available, otherwise it uses its own implementation, inspired from the json module in Python.

One thing is still bugging me: should we do permission checks in WikiRenderer? If yes, for what permission?

in reply to:  11 ; comment:12 by Christian Boos, 11 years ago

Replying to rblank:

One thing is still bugging me: should we do permission checks in WikiRenderer?

It's perfectly fine as it is: the client has already the knowledge of the wiki source text it sends to the WikiRenderer, and during rendering, before adding extra informations to the output (e.g. summary for ticket links), the appropriate fine-grained permissions will be checked the usual way.

Also, I see that you said twice (in comment:11 and in the changeset message of r8885 that the default delay should be set to two seconds, but the change itself makes it go from 2s to 1s… Regardless of the intent, I think this will have to be tweaked anyway based on the feedback we get, and if needed we could still add the "semi-automatic" preview I suggested at the end of comment:3.

in reply to:  12 comment:13 by anonymous, 11 years ago

Replying to cboos:

It's perfectly fine as it is: the client has already the knowledge of the wiki source text it sends to the WikiRenderer, and during rendering, before adding extra informations to the output (e.g. summary for ticket links), the appropriate fine-grained permissions will be checked the usual way.

I was wondering if it would be possible to bypass such checks e.g. by using an [[Include]] macro or something. But you are right, the macro should check for the permissions, so we should be fine. Thanks for the confirmation.

Also, I see that you said twice (in comment:11 and in the changeset message of r8885 that the default delay should be set to two seconds, but the change itself makes it go from 2s to 1s…

Heh, programmer blindness :) Actually, I made the change from 1s to 2s while my commit message editor was open for the first commit, and for some reason I had the impression that Subversion didn't take the change (Mercurial doesn't, for example).

Regardless of the intent, I think this will have to be tweaked anyway based on the feedback we get, and if needed we could still add the "semi-automatic" preview I suggested at the end of comment:3.

I'll still put it back to 2s, as I don't want to overload the server with too many queries. 1s is fine for fast typers, but for "normal" people this will lead to multiple updates within a sentence.

I hadn't forgotten about the semi-automatic preview, but it's made somewhat tricky due to browser incompatibilities as to which event is fired at what time. Still, one thing that could be improved already is to use the delay only for key press events, and trigger an update immediately on blur. I'll experiment a bit with that.

comment:14 by Remy Blank, 11 years ago

(anonymous was me)

comment:15 by Remy Blank, 11 years ago

Auto-preview timeout re-set to 2 seconds in [8900].

comment:16 by Remy Blank, 11 years ago

Resolution: fixed
Status: newclosed

With [8901], the automatic preview is triggered immediately on blur.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.