Skip to:

Opened 13 years ago

Closed 13 years ago

#3574 closed defect (bug) (fixed)

Deleting comment from update - too much spacing

Reported by: xeqta's profile xeqta Owned by:
Milestone: 1.6 Priority: normal
Severity: trivial Version:
Component: Activity Keywords: has-patch


I noticed some od behavior on testbp.

I made a new status update and then a reply. Afterwards I deleted my reply and as the reply is being removed, some alignment issues accour.

(See screenshot)

There's just too much space between the "Comment" button and the border line below.

Attachments (3)

deleting-comment.jpg (126.0 KB) - added by xeqta 13 years ago.
3574.01.patch (1016 bytes) - added by boonebgorges 13 years ago.
3574-2.patch (1.0 KB) - added by DJPaul 13 years ago.

Download all attachments as: .zip

Change History (12)

#1 @xeqta
13 years ago

Also: After a refresh the alignment is correct but it should be fixed even without the need to refresh.

(I only tested this with Firefox on

#2 @xeqta
13 years ago

The spacing I'm taking about between the Comment button and the border is doubled. It needs to be half the spacing to be right.

#3 @johnjamesjacoby
13 years ago

  • Milestone changed from Awaiting Review to 1.5.1
  • Version 1.5 deleted

This has been around for a while. Something to do with the padding/margin on the DOM element after jQuery hides it.

Moving to 1.5.1 milestone to look at after 1.5.

#4 @boonebgorges
13 years ago

  • Keywords needs-patch added

#5 @r-a-y
13 years ago

  • Severity changed from normal to trivial

#6 @boonebgorges
13 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

The root problem is actually pretty simple - the has-comments class on the activity item's <li> element is not removed when the last comment is deleted. See 3574.01.patch for a solution.

I'll note that 3574.01.patch looks like a bigger change than it actually is. In order to make the new_count check at the end as efficient as possible, I wanted to cast the objects as variables rather than traversing the DOM more than necessary. That said, I'm happy to have this punted if others think it's too significant a theme change for a dot-dot release.

13 years ago

#7 @DJPaul
13 years ago

  • Keywords needs-testing removed

Worked for me. I've tweaked the javascript a bit; there's no need to pass existing jQuery collections back through jQuery again, and I've removed a variable as it was only used once.

#8 @boonebgorges
13 years ago

  • Milestone changed from 1.5.2 to 1.6

#9 @boonebgorges
13 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [5348]) Remove has-comments class from activity item when final activity comment is deleted, to help make spacing more uniform. Fixes #3574. Props DJPaul for help with the patch

Note: See TracTickets for help on using tickets.