Edgewall Software
Modify

Opened 13 years ago

Closed 9 years ago

Last modified 3 years ago

#10207 closed defect (fixed)

Clicking 'Submit' or 'Modify Ticket' from comment edit is blocked by comment preview function

Reported by: txcraig@… Owned by: Christian Boos
Priority: normal Milestone: 1.1.3
Component: ticket system Version: 0.12-stable
Severity: minor Keywords:
Cc: Ryan J Ollos Branch:
Release Notes:
  • Moved the Add Comment fields below the Modify Ticket ones. This is both more user-friendly and eliminates the problem of the Submit button being displaced by the apparition of the preview.
  • Moved the author field to the Comment and Properties fieldsets.
API Changes:
Internal Changes:

Description

Here is the scenario:

  • I am adding a comment to a ticket by typing in the textarea (Firefox 4.0)
  • I finish typing my comment and am ready to submit, so use the mouse to click the Submit changes button
  • the wiki-preview of the comment is updated but the ticket is not updated - i.e. I am still in edit mode.
  • I have to click Submit changes again to update the ticket

It took a few times of this happening before I realized that probably the "lost focus" handler that updates the preview was firing just before the button click, and was therefore preventing the submit from happening.

The other note is that this is not 100% reproducible - sometimes the submit goes through, other times it does not. It seems like only the first time the wiki preview is updated the problem exhibits - after that it works as expected.

Trac 	0.12
CustomFieldAdmin 	0.2.2
Docutils 	0.6
Genshi 	0.6
mod_python 	3.3.1
pysqlite 	2.3.2
Python 	2.5.2 (r252:60911, Feb 21 2008, 13:11:45) [MSC v.1310 32 bit (Intel)]
RPC 	1.1.2-r0
setuptools 	0.6c9
SQLite 	3.3.4
Subversion 	1.6.6 (r40053)
jQuery:	1.4.2

Attachments (0)

Change History (27)

comment:1 by Remy Blank, 13 years ago

What happens quite often is that the "Submit" button is pushed down by the preview at the precise moment when I try to click it, so I click on the preview instead. Then I have to scroll down, find the button again and click it.

It's a minor annoyance, and I have taken the habit of waiting for two seconds before clicking "Submit". Also, the progress indicator on trunk should make it more explicit when a preview operation is in progress. Other than that, I don't know what we could do to improve the situation. Suggestions welcome.

comment:2 by txcraig@…, 13 years ago

Thanks for taking a look at this.

pushed down by the preview at the precise moment

It looks like the 'blur' event is the one causing this. Is there any reason why blur is needed as a trigger for the update? Seems like the timeout on the keydown/keypress is sufficient. If blur was taken out, then at least you would vastly reduce the chances that the update happens at the same moment you click the button. You could further reduce the odds by having mousemove reset the timer (as I am moving my mouse down to submit, it ensures the preview is not updated for a bit).

It's a minor annoyance, and I have taken the habit of waiting for two seconds

Agreed it is a minor issue. I never thought of waiting the 2 seconds. For new users especially this might be a bit of a head-scratcher.

the progress indicator on trunk should make it more explicit when a preview operation is in progress.

Aha, I see the progress indicator on this Trac, cool! I don't think it has any effect on this problem though, since the "push down" happens just before the progress indicator starts.

I was wondering why I was not seeing this problem on the Edgewall Trac, then it occurred to me that since my Trac is on the LAN the response time is so much quicker - that must be the difference.

in reply to:  2 comment:3 by Remy Blank, 13 years ago

Milestone: 0.13
Owner: set to Remy Blank

Replying to txcraig@…:

It looks like the 'blur' event is the one causing this. Is there any reason why blur is needed as a trigger for the update? Seems like the timeout on the keydown/keypress is sufficient. If blur was taken out, then at least you would vastly reduce the chances that the update happens at the same moment you click the button.

Not quite. The blur event triggers an update only if the content has changed since the last update, in which case the timer has already been started, and the update will be triggered anyway. It's all a matter of timing. However…

You could further reduce the odds by having mousemove reset the timer (as I am moving my mouse down to submit, it ensures the preview is not updated for a bit).

… that's a very nice idea. I'll experiment with the idea and see if it improves the situation. So this is related to #7145.

Agreed it is a minor issue. I never thought of waiting the 2 seconds. For new users especially this might be a bit of a head-scratcher.

Yes, agreed.

Aha, I see the progress indicator on this Trac, cool! I don't think it has any effect on this problem though, since the "push down" happens just before the progress indicator starts.

I was wondering why I was not seeing this problem on the Edgewall Trac, then it occurred to me that since my Trac is on the LAN the response time is so much quicker - that must be the difference.

You can also tune the delay before the update is triggered. It's set to two seconds here.

comment:4 by Christian Boos, 13 years ago

Is this a Firefox issue only? Never noticed that with Chrome…

Last edited 13 years ago by Christian Boos (previous) (diff)

in reply to:  4 ; comment:5 by Christian Boos, 13 years ago

Ok, indeed, I've just experienced it with Firefox, although it's not easy to reproduce.

For the fix, I'd prefer something deterministic rather than relying on mouse moves or timings. For example, can't we cancel the effect of the preview action when the submit button gets pressed?

in reply to:  5 comment:6 by Remy Blank, 13 years ago

Replying to cboos:

For the fix, I'd prefer something deterministic rather than relying on mouse moves or timings. For example, can't we cancel the effect of the preview action when the submit button gets pressed?

That's a good idea, too. It will solve the issue of the OP, but not the moving buttons.

comment:7 by lkraav <leho@…>, 13 years ago

Definitely thanks to the OP for taking the time to report this annoyance :)

comment:8 by txcraig@…, 13 years ago

can't we cancel the effect of the preview action when the submit button gets pressed

I was under the impression the submit button moved before it ever got a chance to be pressed

thanks to the OP for taking the time to report this annoyance

Thanks to everyone for working on the issue. I really should have attempted a fix myself and proposed a patch, but I don't have a dev environment setup for Trac yet. Hope to do that soon.

comment:9 by txcraig@…, 13 years ago

Summary: Clicking submit from comment edit is blocked by comment preview functionClicking 'Submit' or 'Modify Ticket' from comment edit is blocked by comment preview function

Along with "clicking Submit", "clicking Modify Ticket" exhibits the same issue for me, it scoots out of the way and remains unexpanded. I have to click Modify Ticket a 2nd time to get it to expand. This is a very common case to add a comment and then modify the ticket to update the time spent or other fields.

It's likely that any fix for the Submit button would also fix Modify Ticket, but just to be complete I wanted to note this. Tweaked the Summary to reflect this.

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

Replying to txcraig@…:

Along with "clicking Submit", "clicking Modify Ticket" exhibits the same issue for me, it scoots out of the way and remains unexpanded.

Hm, you may have an "intermediate" trunk version, I suppose. The dynamic preview currently appears below the Modify Ticket heading, so the latter can't jump down like the |Preview| and |Submit| buttons might do.

But this also suggests we could fix the problem by moving those buttons on that line:

+---------------------------------------+
|                                       |
+---------------------------------------+
> Modify Ticket       |Preview|  |Submit|

I think that would also fit well with some pending changes I have for #10012 that style the "actionable" headings a bit like buttons.

We could also duplicate the buttons at the bottom of the dynamic preview:

+---------------------------------------+
|                                       |
+---------------------------------------+
V Modify Ticket       |Preview|  |Submit|

 ....
 ...
 ..

 --------------------------------------- 
| Changed by ...                        |
|                                       |
 --------------------------------------- 
 |Preview|  |Submit|         Attachments^

i.e. only visible when the preview is shown. Note that the second |Preview| button is probably not needed when the automatic preview is active.

comment:11 by Remy Blank, 13 years ago

I have finally experienced the effect explained by the OP. If you have a very fast server, the time between the blur event of the comment <textarea> and the click event of the button is enough for the preview to appear, and the button to be pushed down and therefore the click event doesn't even happen. This is due to the blur event triggering a request directly, instead of triggering the update timer. I will change that so that all events just trigger the timer, and this should fix the original issue.

I will also experiment with duplicating the buttons as suggested in comment:10.

comment:12 by Remy Blank, 13 years ago

Changed the blur event to generate a delayed request in [10742].

comment:13 by Remy Blank, 12 years ago

Milestone: 1.0next-major-0.1X

Moving "experimental" stuff post-1.0.

comment:14 by Christian Boos, 11 years ago

Yet another approach… move "Modify Ticket" above "Add Comment".

The other day I got this suggestion from people not so familiar with Trac, and I think it makes sense. I was about to open an enhancement request ticket when I remembered about this one.

Now that the "Modify Ticket" is always folded, the default view doesn't show much differences one way or the other. When the "Modify Ticket" part of the form is expanded, having to add the comment below makes it more natural (the change and then the explanation for the change, like in wiki edits), and we avoid the problem of things jumping around when the auto-preview gets updated. Finally, by having the buttons just between the comment textarea and its preview, we're consistent with the "Edit Comment" in-place layout.

Feedback welcome on:

comment:15 by txcraig@…, 11 years ago

I agree that moving "Modify Ticket" above "Add Comment" makes sense.

I was about to open an enhancement request ticket

Shouldn't it be a separate enhancement request? What if someone wants to debate the merits of moving the UI around? It might get confusing if that discussion takes place in this ticket about the blur bug. Maybe I am being too strict :)

comment:16 by Christian Boos, 10 years ago

Milestone: next-major-releasesnext-dev-1.1.x

While upgrading the demo-1.1 site where the patch from comment:14 was still applied, I rebased it (see [f58fb53d/cboos.git]). I still like it, I'd still like to have some feedback from other maintainers before applying it!

in reply to:  16 ; comment:17 by Peter Suter, 10 years ago

the patch from comment:14 … I rebased it (see [f58fb53d/cboos.git]).

I find it difficult to give comprehensive informed feedback here. There seems to be a lot of previous discussion and strong user opinions in this area to consider. But much of that would seem off-topic in this ticket.

Subjectively, just after playing with it a bit I like it a lot.

I also quite like the idea of merging the anonymous Author section into the Add Comment' section. But the Add/Remove from Cc: <Author field> checkbox becomes a bit puzzling.

Version 0, edited 10 years ago by Peter Suter (next)

comment:18 by Remy Blank, 10 years ago

Same here, "Modify Ticket" above "Add Comment" feels more natural, and "Author" as part of "Add Comment" looks good. +1 to the change.

in reply to:  17 comment:19 by Christian Boos, 10 years ago

Milestone: next-dev-1.1.x1.1.2
Owner: changed from Remy Blank to Christian Boos
Release Notes: modified (diff)
Status: newassigned

Replying to psuter:

I also quite like the idea of merging the anonymous Author section into the Add Comment section. But the Add/Remove from Cc: <Author field> checkbox becomes a bit puzzling.

Improved that with:

Besides, I also restored the reporter field which got removed by accident:

The anonymous user creating a ticket will now also see the reporter field in the same place as a ticket admin, i.e. as a field just below the Summary: field. The label will be different though (Your email or username) and the hint about Preferences will be there as well.

Last edited 10 years ago by Christian Boos (previous) (diff)

comment:20 by Ryan J Ollos, 10 years ago

The changes look good to me. I rebased on the trunk in log:rjollos.git:t10207-add-comment-at-bottom.4? Should we just go ahead and push these for 1.1.2, or wait for 1.1.3. If we wait, it would be good to push them at the start of the milestone so that we have time to get feedback.

I wish there was something we could do with the hint E-mail address and user name can be saved in the Preferences, since it makes the page look a bit cluttered. I don't think there is anything that can be done short of a major change such as adding a small help icon that displays the text on hover, or using HTML5 placeholders. In implementing either of those features we lose the ability to have a link to preferences though.

#11466 suggests merging the ticket properties display and edit boxes. That would be a good step towards simplifying the page well beyond anything that can be done in the current state.

Last edited 9 years ago by Ryan J Ollos (previous) (diff)

comment:21 by Ryan J Ollos, 10 years ago

Cc: Ryan J Ollos added

comment:22 by Ryan J Ollos, 10 years ago

Milestone: 1.1.21.1.3

I'll commit the change at the start of 1.1.3 if no one objects.

comment:23 by Ryan J Ollos, 9 years ago

Additional changes in log:rjollos.git:t10207-add-comment-at-bottom.5.

  • Removes the checkbox when email is not set in Preferences.
  • author input spans the width of the form.

comment:24 by Ryan J Ollos, 9 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

#11804 was created to deal with a subset of the changes proposed in comment:23.

The other changes were committed to the trunk in [13211:13212].

comment:25 by Ryan J Ollos, 9 years ago

Release Notes: modified (diff)

comment:26 by Ryan J Ollos, 9 years ago

Reduced number of translatable strings in [13221].

comment:27 by Ryan J Ollos, 9 years ago

comment:14:ticket:10841 describes a regression from this ticket.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos 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.