Linebreaks are lost in comments

[oliverlloyd]

When previewing a comment or viewing it after it has been optimistically rendered additional line breaks are not shown. These missing breaks do appear when the page is refreshed and the server sent content is rendered.

[oliverlloyd]

Raised by moderators based on reader feedback

[bryophyta]

It looks to me like there might be two separate issues here.

1. Styling on preview comments

The markup for the comment contents appears to be the same for both the preview and server-rendered comments. Newlines in the comment input are being translated into a combination of <br> and <p> tags. The difference in appearance seems to just come from the fact that the contents in the non-preview version are wrapped in a parent which adds a bottom margin to <p> tags:

  css-kli0yq-Comment p {
      margin-top: 0px;
      margin-bottom: 12px;
  }

This style is missing from the preview wrapper (the wrapper for the preview is in fact a <p> tag itself, which should perhaps be addressed, too).

2. Markup on client-rendered comments

The client-rendered version of a submitted comment (i.e. what appears after submission, but before refreshing the page, seems to just be a plaintext version of the comment contents, with newlines preserved as newlines, hence not rendered as line breaks by the browser.

Screenshots

Preview

<p class="css-12jyz7u"><p>hello world<br>linebreak</p> <p>another line break</p></p>

Client-rendered (after submission)

<div class="css-kli0yq-Comment">Hello world!
this is a line break

this is two


this is three</div>

Server-rendered (after submission)

<div class="css-kli0yq-Comment"><p>Hello world!<br>this is a line break</p> <p>this is two</p> <p><br>this is three</p></div>

[bryophyta]

Update

Preview rendering

@mxdvl and I think we've resolved the first issue identified above, by adjusting the styling on DCR.

Optimistic post-submission rendering

Issue 2 from above (formatting for the 'optimistic client-side rendering of submitted comments) requires us to get the appropriate markup for the comment. Two possible options:

• Make another call to the comment/preview/ endpoint after the comment is submitted, and use the returned markup to allow us to the render comment prior to a page refresh.
• Adjust the comment submission endpoint (/discussion/:key/comment/ to return the comment markup, like the preview endpoint does. This could then be used to render the comment after submission, but without a page refresh.
  The tradeoff seems to be between adding an extra call vs. making changes to a separate codebase (i.e. the discussion-api).

For reference, I think that the relevant parts of that codebase are:

Creating the markup for the preview endpoint:
https://github.com/guardian/discussion-api/blob/5cd031be91e47fcbce6d3473ca875ceadb98ae33/discussion-api/src/main/scala/com.gu.discussion.api/api/CommentApi.scala#L71-L84

Generating the response for the new comment endpoint:

https://github.com/guardian/discussion-api/blob/5cd031be91e47fcbce6d3473ca875ceadb98ae33/discussion-api/src/main/scala/com.gu.discussion.api/api/DiscussionApi.scala#L188

(I might be totally off here though, as this is my first time looking at that codebase.)

[bryophyta]

Update on issue 2

For the optimistic rendering, it looks as though this is done here: https://github.com/guardian/discussion-rendering/blob/78b97d5ad27f1579a9144edcd8f144a0a3591a3c/src/components/CommentForm/CommentForm.tsx#L325-L334

The value of body that's currently passed to simulateNewComment is just plain text, which is why the optimistic renderings don't have the right spacing. If we can get the marked-up version of the text here then it looks like it should be rendered more accurately.

Questions:

• Is it worth adding an extra API call to the preview endpoint in order to get the right markup? If so, should we set the extra call going at the beginning of the submission process, or only once the submission has succeeded?
• The submitForm function linked above has a defaultComment method which can be overridden. If it was overridden, would this have any implications for this post-submission rendering issue?
• And are there any situations where a user of the rendering app wouldn't want the preview markup to be rendered? (Does anyone else use discussion-rendering, or is it just for DCR?)

@oliverlloyd do you have any thoughts on this?

[oliverlloyd]

Really great write up and investigation @bryophyta , thanks!

• Adjust the comment submission endpoint (/discussion/:key/comment/ to return the comment markup

I think this is the best, most semantic, option. It is an additional change to a different codebase but you can take the rose tinted shiny happy view that downloading and learning a new codebase is a positive thing? Adding an extra api call on the client is a worse reader experience and is going to add complexity to the code.

If we can get the marked-up version of the text here then it looks like it should be rendered more accurately

Is this possible without changing the discussion-api? I do think that having the api return the html for the resulting comment is actually a nice solution and might be good in any case but if we can get away with solving the issue purely on the client that has advantages.

And are there any situations where a user of the rendering app wouldn't want the preview markup to be rendered? (Does anyone else use discussion-rendering, or is it just for DCR?)

At this point it is just DCR consuming discussion-rendering. But both Frontend and DCR consume discussion-api (we still load discussion on some articles that have not yet been rendered, e.g., crosswords.)

[bryophyta]

Okay that's really helpful to hear, thanks!

I agree that changing the API response would be nicest in many respects. My hesitation was partly due to not knowing how long it will take to make the edit, and also not knowing whether it could constitute a breaking change for any other consumers of the API. But from your comments it sounds like it might be worth spending the time to work these things out.

Do you happen to have a sense of how we could find out whether adding a field to the /discussion/:key/comment/ endpoint response might be a breaking change for any other consumers? (I think apps also uses discussion-api, right?)

[mxdvl]

Usually, adding more information isn't a breaking change. As long as the old keys are still present, there should be no problem with consumers.

[bryophyta]

This probably shouldn't have been closed, as part of the issue is still unsolved. The solution outlined above to the second part of the issue looks viable, but it requires work on the discussion API. Currently this ticket is in the backlog.

[rhiannareechaye]

We are deprioritising this ticket by 'closing a not planned' - this won't delete the ticket and it will be searchable if the issue crops up again. Unfortunately we have over 130 health tickets to address in the web team and we can't address them all. With so many tickets we are potentially missing things that are high-impact so we're reorganising.

[mxdvl]

We’ll be revisiting the broken preview as part of the discussion work in #10023