Update comment responses count when adding replies #4003
Conversation
Senen
force-pushed
the
fix-comments-reply-alternate
branch
from
66369e0
to
096eb73
Compare
Senen
force-pushed
the
fix-comments-reply-alternate
branch
from
096eb73
to
c24597c
Compare
Senen
force-pushed
the
fix-comments-reply-alternate
branch
from
c24597c
to
66b2afb
Compare
Senen
force-pushed
the
fix-comments-reply-alternate
branch
2 times, most recently
from
51567d9
to
6234553
Compare
Senen
force-pushed
the
fix-comments-reply-alternate
branch
from
6234553
to
343a1c5
Compare
app/assets/stylesheets/layout.scss
Outdated
|
|
||
| .responses-count { | ||
| .far { | ||
| @extend .fa-minus-square; |
There was a problem hiding this comment.
Prefer using placeholder selectors (e.g. %some-placeholder) with @extend
There was a problem hiding this comment.
Unfortunately font-awesome doesn't provide mixins nor placeholder selectors for these styles. We could write:
.far::before {
content: fa-content($fa-var-minus-square);
}But, to be honest, this feels like copying the body of a method instead of calling the method. I'd rather use @extend, even if we aren't following the rules 😌. On the other hand, having style warnings is annoying 😅. @Senen What you decide is fine with me 😉.
Senen
force-pushed
the
fix-comments-reply-alternate
branch
from
ef4915c
to
8bf8726
Compare
app/assets/stylesheets/layout.scss
Outdated
|
|
||
| .responses-count { | ||
| .far { | ||
| @extend .fa-minus-square; |
There was a problem hiding this comment.
Unfortunately font-awesome doesn't provide mixins nor placeholder selectors for these styles. We could write:
.far::before {
content: fa-content($fa-var-minus-square);
}But, to be honest, this feels like copying the body of a method instead of calling the method. I'd rather use @extend, even if we aren't following the rules 😌. On the other hand, having style warnings is annoying 😅. @Senen What you decide is fine with me 😉.
Senen
force-pushed
the
fix-comments-reply-alternate
branch
from
8bf8726
to
9951ec0
Compare
| margin-top: $line-height / 2; | ||
| &.collapsed { | ||
| .far { | ||
| @extend .fa-plus-square; |
There was a problem hiding this comment.
Prefer using placeholder selectors (e.g. %some-placeholder) with @extend
Senen
force-pushed
the
fix-comments-reply-alternate
branch
from
9951ec0
to
4bc3d82
Compare
Extract the needed portion of code to a new partial to be able to update only the elements needed when a new comment is added keeping UI properly updated.
When a user replies to a comment whose responses was hidden at the moment of reply form submission and although the reply were correctly added to the DOM it was hidden because was added to a collapsed list. This solution is about showing all responses of parent comment after adding the new comment to the DOM so the user can see new reply into the screen. (This is not applicable to root comments which cannot be collapsed)
Co-Authored-By: Javi Martín <javim@elretirao.net>
Senen
force-pushed
the
fix-comments-reply-alternate
branch
from
4bc3d82
to
31c0b43
Compare
|
Travis failure is not related to this pull request and will be solved by #4023. |
References
This PR is an alternative solution of the problem described here #4002.
Objectives
Update the parent comment "responses count" when a new reply is added.
The solution is about replacing parent comment responses count (which is also the toggler to show/hide children comments) when a new reply is added.
Changes introduced at 7105f21 (the comments part) are needed to initialize automatically comments added or replaced through ajax calls.
Steps to reproduce
At this point you can see your reply but parent comment responses count is not updated!
Visual Changes
Before
After
