search

67P / hyperchannel

Kosmos Chat for the Web
Mozilla Public License 2.0
20 stars 3 forks source link

Add support for bold and italic messages #101

Closed gregkare closed 7 years ago

gregkare commented 7 years ago

Connected to #12

I have used spans with classes, I'm open to other ways to do it though

raucao commented 7 years ago

Cool. I think we should use <em> for italic and <strong> for bold instead. Those are the semantic HTML equivalents to the classes.

gregkare commented 7 years ago

Thanks, changing it

gregkare commented 7 years ago

I should have tried it, I used spans because I thought the <em> and <strong> tags would just be open and I'd have to close them manually :)

gregkare commented 7 years ago

Adding the semantic tags and trying to close them using replacement didn't work at all, the results were super unpredictable. In a342c16 I have switched to an npm module to do the actual replacements, dealing with nesting and resetting properly. I couldn't find a way to use <strong> and <em> tags instead of <span> (because Handlebars cannot do comparisons like equality without helpers, and I couldn't make any of the solutions work)

Now it supports bold, italic, underlined and proper nesting:

screen shot 2017-05-04 at 15 20 20
raucao commented 7 years ago

As discussed in AFK conversation yesterday, we don't actually want underlines to be used with something that's not a link, and we add our own underlines when turning URLs into actual links in the message component. Underlines really only make sense to style links in text-only IRC clients, where they're normally not underlined.

This worked almost with our own implementation that was extremely simple and didn't rely on a 3rd-party module (which will have to be included in the app code). I think we should go back to our solution, use spans instead of semantic elements for now, and remove underline support. No external dependencies, no additional code bloat and supports all the features we need.

raucao commented 7 years ago

Looks like @gregkare missed my last comment, but he's on vacation for a week from today on.

I'll check if I can revert to an earlier commit and just remove the underline.

raucao commented 7 years ago

@gregkare Wanna have a look at this again? Seems like we just need to pick a previous commit.

gregkare commented 7 years ago

Ack, will do!

gregkare commented 7 years ago

I have pushed the simplest change to support bold and italic and removed underlined support. It still has the issue of the style reset only resetting one tag, but I don't see a way to do this properly without using a real IRC style parser, so this is good enough

raucao commented 7 years ago

Great!