Problem/Motivation

Click on a 'new' comment marker via a view configured to show it. There's no anchor.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 interdiff_3270574_3-5.txt906 bytesandregp
#5 3270574-5.patch1.08 KBandregp
#3 3270574-3.patch890 bytescatch
#2 3270574.patch670 bytescatch

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 3270574.patch670 bytes

This is whats missing. It should not really be necessary for all this to be in the template...

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new 3270574-3.patch890 bytes

Comment js also relies on the js-comment class.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

This is a great find! A couple issues:

1) I really didn't understand what was happening until I looked at Bartik and read the code comment within that template. Lets add the same comment into the Olivero template.

  {#
    Hide the "new" indicator by default, let a piece of JavaScript ask the
    server which comments are new for the user. Rendering the final "new"
    indicator here would break the render cache.
  #}

2) I don't believe the <mark> element is semantically correct here. According to MDN

The <mark> HTML element represents text which is marked or highlighted for reference or notation purposes, due to the marked passage's relevance or importance in the enclosing context.

This isn't what we're doing. This element is simply giving JavaScript something to ingest.

andregp’s picture

andregp commented
Status: Needs work » Needs review
StatusFileSize
new 3270574-5.patch1.08 KB
new interdiff_3270574_3-5.txt906 bytes

Fixed patch according to #4

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Reviewed & tested by the community

#5 looks perfect. Pretty sure this patch should apply for d10... adding a test.

I verified this by creating two users, and a view that shows new comments. When user 1 created a comment, the "new" link was visible to user 2 and hyperlinked properly.

  • lauriii committed c668e0b on 10.0.x 
    Issue #3270574 by catch, andregp, mherchel: Olivero comment template...

  • lauriii committed 4766b71 on 9.4.x 
    Issue #3270574 by catch, andregp, mherchel: Olivero comment template...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Should we open follow-up to change the <mark> element to a <span> in the comments module?

Committed c668e0b and pushed to 10.0.x. Also cherry-picked to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.