168

I noticed that the edit-dialogue for comments has changed. Now when you click on edit a new area will be opened and the comment will be kept so we can see the old version while editing it. This can be a good feature1

BUT while editing the edit link remains clickable and we can click it as many times as we want!

Here is the result, a crazy thing:

Screenshot showing four textareas below a comment, the number of characters left available for the comment, three more textareas that also have their own Save button, cancel and help links, and characters left count, and two more textareas below that again with their own buttons, links, and count.

I think we need somehow more jQuery to disable the edit link after the first click or keep the old edit feature where the old comment disappeared while editing.


1 : maybe this is the main bug and it was not intentional

34
  • 2
    am testing the edit comment here (same issue !) Jul 20, 2018 at 11:14
  • 5
    When you hit save all the boxes go away. And only the changes in the top box are saved. Jul 20, 2018 at 11:15
  • 3
    @AndréKool I also like the pyramid logic when you keep clicking .. .at the bottom 1 empty block, above it 2 empty then 3 empty, etc :p Jul 20, 2018 at 11:20
  • 92
    Comment box, go home, you're drunk.
    – MonkeyZeus
    Jul 20, 2018 at 11:54
  • 2
    testing the edit box(replicated) Jul 20, 2018 at 11:56
  • 5
    * your issue is not height :) Jul 20, 2018 at 12:15
  • 5
    @HereticMonkey yes took the screenshot before the edit :p and at the end forget the edit :/ Jul 20, 2018 at 12:17
  • 31
    This website is called StackOverflow for a reason. This CommentOverflow is a feature of this website. status-by-design. Jul 20, 2018 at 16:34
  • 23
    The first box wins the boxing match.
    – Taplar
    Jul 20, 2018 at 16:45
  • 4
    LOL, almost all of the comments on this question say they have been edited. EDIT: This glitch still happens for me!
    – Luke B
    Jul 20, 2018 at 17:21
  • 6
    Have an upvote for noticign this. It is like noticing that your radio flickers if you open the trunk while going backward at 60 mph.
    – Yunnosch
    Jul 20, 2018 at 19:33
  • 12
    They really need to test these changes internally before they push them out to us...
    – TylerH
    Jul 20, 2018 at 20:26
  • 15
    @TylerH why test before deploying if you have tens of thousands of drones testing anyway? :P Jul 21, 2018 at 10:17
  • 8
    Somebody decided to break comments in general. First the arrows, then the pencil color and now more boxes.
    – Victoria
    Jul 21, 2018 at 22:42
  • 4
    I have a bug fix in the code review queue. Hopefully it'll get merged tomorrow. Aug 7, 2018 at 0:54

2 Answers 2

8

This is fixed.

There were two problems at play here. The first is that we weren't hiding "edit" button, or really the whole comment.

Backstory: We are in the process of moving to a structured design system called Stacks which includes atomic classes like d-block and d-none. These classes are meant to act as overrides of component styles and have !important on them. Unfortunately, this doesn't play well with jQuery's $.fn.css-based functions, like $.fn.hide.

What happened: Someone was editing the comment HTML, saw a stray style="display: block" on a div, and changed it do class="d-block". Then when you clicked on "edit", the bravest JavaScript in the world, jComment.find(" > * > *").not(jForm).hide();, tried hiding the element and failed, because !important is important!

The fix: I just reverted that line of code. A thought out restructuring of the JavaScript is probably more appropriate, but there are already plans to overhaul the comment UI so I'm just going to wait for that to do JavaScript changes.


The second problem was why you got so many edit fields, ΣN of them where N is the number of clicks. Assuming someone is going to break thing again, we've got to fix that.

What happened: This problem was less nuanced. The comment.edit function just wasn't idempotent, so calling it multiple times yielded multiple inputs. The increasing number of fields is because we do two inserts, first we insert the new container, then we insert inputs, and the second insert was matching both the new and the old containers.

The fix: I just checked to see if the input was already there before adding it again.

5
  • 3
    this a test comment. edit: yes it's fixed! :p Aug 7, 2018 at 19:12
  • @TemaniAfif $('#comment-618685 .comment-body').addClass('d-block') if you want to recreate the first issue to test the second. Aug 7, 2018 at 19:15
  • As a side note : is it really necessary to have the display:block initially? it's already a div element so why it's needed? Aug 7, 2018 at 19:15
  • 1
    I'm pretty sure it was just put there as a hint that the style would change, ironically setting up its own demise. Aug 7, 2018 at 19:15
  • Ok make sense ;) and yes the second issue seems ok too Aug 7, 2018 at 19:18
52

I found the bug, and it's related to CSS. The JavaScript code is working fine and I see a display:none added to the initial comment in order to hide it, but there is another CSS that is overriding the inline one:

Enter image description here

It seems an important issue and we should get rid of that !important or add another !important to the inline style.

By the way, better avoid !important. This is a clear side effect of its use.1


An idea of the fix is to add the class d-none to the element instead of adding the inline style. This one appear later in the CSS and it also uses !important, thus it will fix the issue.

By the way, I am not sure if the use of d-block class there is justified. It seems useless since the element is a div and there is no other style changing its display. If we remove it the issue will also get fixed without altering the JavaScript code.


1: Of course, I am not here to judge or criticize the great work done by the developers. It's a simple opinion, and I know it's more complex than simply removing an !important.

7
  • 4
    It's never complex if you use the power of JQuery!
    – Jesse
    Jul 21, 2018 at 11:36
  • 11
    !important is a hack anyway and should never be found in production code. Jul 22, 2018 at 17:07
  • 6
    @LightnessRacesinOrbit well, it's not easy when it comes to library and frameworks, In such cases, we are obliged to use !important ... The most common example is the Bootstrap framework where you will find more than 500 !important in their CSS. And here they are using the Stack CSS where there is more than 1000 !important .. yes it's too much ... Jul 22, 2018 at 18:24
  • 6
    Just googled that and found that !important is not not important but rather very important. It should really be important! instead. Maybe someone can fix that with some jQuery powers. Jul 22, 2018 at 20:43
  • 3
    @TemaniAfif I would assume that when you are building a CSS framework for a very particular site, like Stack CSS is doing for SO and the SE network, then you should be able to design the CSS a bit better than having to rely on !important rules.
    – poke
    Jul 23, 2018 at 9:35
  • @poke it seems easy said like that but when building a framework you will have many class utilities and for those classes you are obliged to rely on !important .. the framework need to be accurate and follow a particular logic because the users of this framework will only rely on adding classes. You can simply check the well known and most used CSS framework (bootstrap) and you will see that there is a plenty of !important and it's very hard to avoid them. Jul 23, 2018 at 9:47
  • 1
    @TemaniAfif It may not be easy, but it's still the right thing to do :) On the topic of Bootstrap, I did notice !important in their code during my last project, and was aghast. Jul 23, 2018 at 11:41

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .