content: Handle <hr> horizontal lines

[Khader-1]

Final Result:

Fixes: #353

[chrisbobbe]

Thanks, this looks mostly good! Some small comments, below.

Also, let's add a some test for the new ThematicBreak widget, in test/widgets/content_test.dart. One shortcut to doing that is testContentSmoke, but unfortunately it won't help us here—it requires ContentExample.expectedText to be non-null, and these "thematic break" lines don't come with any text to check. 🙂 But there are some tests in that file in the same situation, and they end up being pretty simple. For example, the smoke tests for MessageImageEmoji.

[Khader-1]

Thanks @chrisbobbe! I have restructured the code accordingly.

[gnprice]

Thanks @Khader-1, and thanks @chrisbobbe for the review!

A few more comments below, mainly on the tests.

Please also update the screenshot at the top of the PR for the current revision (since I believe you've adjusted the layout). That'll help for getting feedback on the visual result.

[gnprice]

Thanks for the revision! Comments below.

[gnprice]

Huh, is this the HTML the server outputs for this input? I'd have guessed there'd be some newlines in there, like between </p> and <hr>, based on how some other examples in this file look.

Would you link in the PR description to a test message on chat.zulip.org that demonstrates this example? (That's probably something we should start making a practice of, for better reproducibility. Or perhaps even put the link into the source code.)

[Khader-1]

Apologies for the oversight. I adjusted the output for readability purposes, which resulted in a cleaner HTML string. However, I understand the importance of maintaining accuracy for testing purposes. Moving forward, I'll ensure the test output aligns precisely with the server's actual output.

[Khader-1]

Here's the link to the message in the latest version of the test:
https://chat.zulip.org/#narrow/stream/7-test-here/topic/content.3A.20Handle.20Thematic.20Breaks/near/1774718
Do you suggest having it in ContentExample as an nullable field? or just a comment will do?

[gnprice]

Cool, thanks. Yeah, readability is important, but for these tests their job is to make sure we're successfully parsing whatever the server sends us, with all its quirks and sometimes messes — so the top priority is for the data to represent those authentically.

For now let's just add the link in a comment, like this:

  static const thematicBreaks =  ContentExample(
    'parse thematic break (<hr>) in block context',
    // https://chat.zulip.org/#narrow/stream/7-test-here/near/1774718
    'a\n---\nb',
    '<p>a</p>\n<hr>\n<p>b</p>',

Then I'll file a separate issue for filling that in for the existing examples, adding it as a field, and also adding some automated validation of it.

[gnprice]

Filed #617.

[gnprice]

Thanks for the revision! Two wording nits below, and I replied above about the URL. I'll make those three tweaks and merge.

[gnprice]

nit: there's only one of these, so singular:

Suggested change

	static const thematicBreaks = ContentExample(
	static const thematicBreak = ContentExample(

[gnprice]

nit: "in block context" is redundant because that's the only way <hr> can appear:

Suggested change

	'parse thematic break (<hr>) in block context',
	'parse thematic break (<hr>)',

(there's a phrase like that for the test that produces LineBreakNode, but that's to contrast it with the "inside a paragraph" version that produces LineBreakInlineNode)