nextcloud / spreed

🗨️ Nextcloud Talk – chat, video & audio calls for Nextcloud
https://nextcloud.com/talk
GNU Affero General Public License v3.0
1.63k stars 437 forks source link

Basic text formatting to chat #1027

Closed theel0ja closed 1 year ago

theel0ja commented 6 years ago

I would like to have at least basic text formatting for chat, such as



Spreed app

Spreed app version: Talk 3.2.2

Server configuration

Nextcloud Version: 1.0.4

nickvergessen commented 6 years ago

I think most other things have bold and italic, so that's okay. Not sure about code tags thou. THat will cause big trouble when we integrate with other services.

nickvergessen commented 6 years ago

Checked the messengers which I use:

IM bold italic striked code
Whatsapp * _ \~ ```
Threema * _ \~ :heavy_multiplication_x:
Telegram ** _ _ :heavy_multiplication_x: ```
Signal ✅ (WYSIWYG) ✅ (WYSIWYG) ✅ (WYSIWYG) ✅ (WYSIWYG)
nickvergessen commented 6 years ago

Survey: Should we support * as bold?

nickvergessen commented 6 years ago

Survey: Should we support _ as italic?

nickvergessen commented 6 years ago

Survey: Should we support ~ as striked?

nickvergessen commented 6 years ago

Survey: Should we support ``` as code?

fancycode commented 6 years ago

Survey: Should we support ``` as code?

Backticks are bad on a German keyboard layout, but they are also consistent with markdown.

mario commented 6 years ago

Why not add a full markdown support while we're at it? :)

nickvergessen commented 6 years ago

Why not add a full markdown support while we're at it? :)

Because at somepoint we would like to integrate with other services (see https://github.com/nextcloud/spreed/labels/feature%3A%20integration ) which is highly difficult then

tcitworld commented 6 years ago

integrate with other services which is highly difficult then

Converting Markdown content into plain text shouldn't be that hard, no ?

theel0ja commented 6 years ago

I'd add most stuff, but for example not headings because they aren't needed in chats.

nickvergessen commented 6 years ago

Well as for markdown, apart from the things in the poll the following are missing:

fancycode commented 6 years ago

I think you can skip lists and headlines. If people want to write documents, they should not use the chat but some editor app :speak_no_evil:

jospoortvliet commented 6 years ago

Lists do make sense, though, I use it also in chat... But certainly bold, underline, strike-through and code are much more important. Headlines do seem weird yes.

ryanprior commented 5 years ago

With regard to https://github.com/nextcloud/spreed/issues/1027#issuecomment-408828927 (suggestion for full Markdown support)

I don't like the idea of having people post links (including potentially misleading ones,) images, or headlines to chat.

Syntax compatibility with markdown might be the most appropriate, but "full Markdown" seems like an own goal.

ryanprior commented 5 years ago

Also, info for @nickvergessen & others who prioritize issues: this is one of the major blockers keeping me from using Nextcloud Chat. I frequently post code & read other people's code in chat, and need it to be formatted nicely.

I would be interested in working on this issue if it's something you would like outside help on.

lachmanfrantisek commented 5 years ago

What about some quote/citation?

> used in markdown would be useful for that:

Example:

> something

something

nickvergessen commented 5 years ago

I would be interested in working on this issue if it's something you would like outside help on.

No objection at all. Everything that is not actively being worked on can be picked up. The main issue I see is:

  1. This needs to also work on iOS/Android
  2. It should be mostly done in the UI so third party integration works as good as possible
  3. There is a difference between markdown and all other chat clients how to do bold/italic
  4. The next thing on our todo is to bring the UI to VueJS (https://github.com/nextcloud/spreed/issues/1347) and that might heavily conflict with any work being done, because we are basically redoing the UI.
ryanprior commented 5 years ago

How is the Android/iOS app built right now? I've never done mobile app development so I'm pretty ignorant of that side of things.

As far as building it into the UI and making it Vue proof, we might build it as an embedded Vue component right away without waiting for the rest of the UI to be re-done. My understanding is that Vue components can play well next to other things and it's not all-or-nothing.

tiotrom commented 5 years ago

I would love to see parts of the markdown being implemented too. Especially for code or quotes. Is one thing that I find it as a "must" for a text-chat.

Gadgitmatic commented 5 years ago

Formating using keyboard shortcuts (CTRL+B, CTRL+U. etc) will apply as expected in the preview box, however, it does not carry formatted once submitted into Talk's conversation area. GIF screen capture below.

Peek 2019-11-12 00-06 Sorry for the flicker. I promise it's not subliminal messaging.

Tested on Nextcloud 17.0.1 Chromium Version 78.0.3904.97 (Official Build)

boppy commented 4 years ago

I would love to see parts of the markdown being implemented too. Especially for code or quotes. Is one thing that I find it as a "must" for a text-chat.

Totally in for that. But I'm also all in for full markdown support. YES, it makes no sense to have links and or images available in two ways, BUT: markdown seems to me as a standard nowadays - for most devs at least. So why not supporting it? It would be easily integrated into apps and services, because there are tons of middlewares available. No need for a new parser - okay, yes, this is also true for stripped down markdown... ¯_(ツ)_/¯

Survey: Should we support ``` as code?

Triple is multiline. Might be intentional, might be a wrapping flaw by github (three single backticks do not convert to one single backtick shown as code). But I would go for a single backtick for inline code (and also triple for multiline).

That would be the most complete and useful input to me (from slack):

image

ltr:

reduced image

small (as reply) w/ menu image

Formating using keyboard shortcuts (CTRL+B, CTRL+U. etc) will apply as expected in the preview box, however, it does not carry formatted once submitted into Talk's conversation area.

For me on latest chrome dev I have it as <html> transmitted.

tromlet commented 4 years ago

Survey: Should we support ``` as code?

I would recommend `` as multi-line code, and as a single line code.

Honestly, Nextcloud Talk strikes me as more of a competitor to Mattermost/RocketChat/Slack/Teams than to, say, Signal etc. The former are more corporate-centric, managed, productivity chat applications... (hence the desire for powerful formatting options right at the chat line) the latter are more personal/individual chat applications. Although as a paranoid open-source fanboy, hook me up with those off-the-record end-to-end encrypted chats with perfect forward secrecy, fam. Anything that makes FBI directors meltdown in front of Congress. :)

Like Slack, it would be nice to get the ability to add text files in-line, and add syntax highlighting to them based on what they are (bash, PowerShell, Ruby, etc).

lists - do you really need lists in chat messages?

Absolutely. I used them all the time at my previous company when I was in Slack, because they're an easy, nicely-formatted way to just blast some quick 'n dirty instructions to a user. Most of my users were non-techie, so being able to easily make a nice, step-by-step set of instructions on how to reset their gate controller or whatever was super nice. Slack made it quite nice, too, it could tell when I was making a list and would begin auto numbering or auto-bulleting for me as I entered in a line and then hit Shift + Enter to the next line.

To add to the list of somewhat advanced features that I'd love to have available in in-line chat, I'd say tables, too. I asked the Slack team for this, and while they were very nice about it, they most likely concluded that I was an edge case and that this was a "maybe someday" feature request lol.

headlines - see lists

I'm with you on this one. Headlines do seem a little out of scope - if you find you need a headline... it's time for a change of tool, something like an email or a document of some kind.

EDIT: Check out Slack's formatting: https://slack.com/help/articles/202288908-Format-your-messages

Cacodaimon commented 4 years ago

I am not in nextcloud development and architecture, but would it be possible to develop a plugin (as app available in the app store) for Talk which adds e.g. Markdown support?

nickvergessen commented 4 years ago

Not really, because the mobile apps would still not understand it and show ![image](url to image) then instead of an image. Also instead of implementing an additional addon, a PR to this app here would be enough.

Cacodaimon commented 4 years ago

@nickvergessen I see, but if we ignore the fact that the apps would show plain markdown instead of formatted content would it be possible to build such a plugin which would work in the web / browser version only?

Maybe the formatting could be stripped (e.g. Bold becomes Bold) for apps consuming the API?

nickvergessen commented 4 years ago

Well all apps and the browser use the same API. But that is also not an issue. If we want this we just need to do it properly and tell the mobile apps to also understand it.

Cacodaimon commented 4 years ago

@nickvergessen I see so a hint when fetching messages e.g. formattedContent = true which defaults to false could do the trick.

The apps does not know about this hint and gets content without formatting and the browser uses the hint to get formatted or plain markdown content.

nickvergessen commented 4 years ago

Well we just bump the api version in that case, and you use api/v3 for with markdown as ultimatly all clients need to support this.

Cacodaimon commented 4 years ago

What should happen with the V2 API? Should it strip markdown content or would it cause some backward compatibility issues? (Maybe configurable)

The V3 in this case would pass the content as it is to client and the client would handle the formatting?

Any other requirements for an proper PR?

In this case I would set up an dev environment and dig into next cloud development this weekend.

Rik44444 commented 4 years ago

yes basic editing would be great. to add to this thread, hyperlink support (see link above) would also be great

Cacodaimon commented 4 years ago

@nickvergessen Hey I made some progress integrating markdown support, you can review my changes here: https://github.com/nextcloud/spreed/compare/master...Cacodaimon:master .

For easier testing I skipped using RichText as external component ("@juliushaertl/vue-richtext"), maybe keeping the component here makes more sense because many specific styles where needed to reset the NextCloud default styles on HTML elements like lists or italic text… .

The used markdown library (remark-parse) might look a little bit over sized for the task but was needed to preserve the features like placeholders and component support of the old RichText component and render the formatted content with VUE's render method instead of displaying plain HTML.

The API side is still missing (the discussed markdown stripping for mobile apps until they support markdown), I could need some directions where to start in the PHP code.

There are two screen shots of my progress.

Basic markdown

Mention and Quote Support

I skipped @boppy suggestion of an advanced text input component with formatting support, because I think that this could be added independently of the basic MD support here.

Please give me your opinion if I am on the right way of an mergeable pull request.

nickvergessen commented 4 years ago

I'm not too familiar with markdown libs, but the Text app seems to use markdown-it. Maybe @juliushaertl can say if that was a good choice? If so I would like the same lib, so people and fixes can be used more efficiently

Cacodaimon commented 4 years ago

@nickvergessen @juliushaertl I have already used markdown-it in another project and it work great, but the downside of markdown-it is that it just renders markdown as plain HTML string (as far as I know).

Hence the original RichText component supports Vue.js components as placeholders e.g. mentions… I had to use a more advanced markdown lib which converts markdown to an AST. This enables extending the syntax (placeholders), choose how to render (via Vue's render method) and more post processing.

The same lib could be used remove markdown where needed, too: https://github.com/nextcloud/spreed/commit/0d291980bb8e58f5652fe91250f29a92c56674f8

juliusknorr commented 4 years ago

I have already used markdown-it in another project and it work great, but the downside of markdown-it is that it just renders markdown as plain HTML string (as far as I know).

There is also a parse method for markdown-it (https://markdown-it.github.io/markdown-it/#MarkdownIt.parse)

The general approach seems good to me, though I'm a bit hesitant towards the larger list of additional dependencies https://github.com/nextcloud/spreed/compare/master...Cacodaimon:master#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R40-R52 so it might be worth to check if this would be something that could also be implemented with markdown-it. However I did not have a detailed look at remark/rehype yet, so the amount of packages is of course not meaningful.

Also I think we could even move the markdown behavior to vue-richtext so other apps can make use of it as well in case we bring markdown support to comments or other areas. What do you think?

Cacodaimon commented 4 years ago

@juliushaertl thanks for your response!

I have already used markdown-it in another project and it work great, but the downside of markdown-it is that it just renders markdown as plain HTML string (as far as I know).

There is also a parse method for markdown-it (https://markdown-it.github.io/markdown-it/#MarkdownIt.parse)

I took a look at markdown-it's parse method, it just returns a list of tokens, so its rather a tokenizer. So I would need to build an AST from the list, the lib 'markdown-it-ast' seems to be unmaintained and does nor produce a valid AST in my test.

Therefore the steps to archive the same would be:

So after my quick research it should be possible to use marokdown-it to archive the same goal, but with more hand crafted code instead of lib usage. The unified lib with it's remark, rehype… plugins is more versatile, it could even used to convert HTML pasted in the chat message input to markdown 🤯. So please make a choice whether to keep my current solution or enforce me to rewrite it to markdown-it ;) .

Also I think we could even move the markdown behavior to vue-richtext so other apps can make use of it as well in case we bring markdown support to comments or other areas. What do you think?

Sure only the following test would fail due to remarks's autolink function, but links with ending braked would work as markdown link.

…maybe keeping the component here makes more sense because many specific styles where needed to reset the NextCloud default styles on HTML elements like lists or italic text…

There is only open question where to keep the styles? Many reset code was needed hence nextcloud's styles overwrites some default HTML styles globally.

juliusknorr commented 4 years ago

So after my quick research it should be possible to use marokdown-it to archive the same goal, but with more hand crafted code instead of lib usage. The unified lib with it's remark, rehype… plugins is more versatile, it could even used to convert HTML pasted in the chat message input to markdown exploding_head. So please make a choice whether to keep my current solution or enforce me to rewrite it to markdown-it ;) .

Ok, that definitely sounds reasonable to me. That would probably something worth to check if we can make use of the same libs in text as well, since implementing a parser with the markdown-it capabilities also seemed like a a lot of custom implementation effort, so we are using some dirty workarounds over there right now.

Sure only the following test would fail due to remarks's autolink function, but links with ending braked would work as markdown link.

I personally prefer that behaviour however the main concern @nickvergessen had about this is that this breaks for example wikipedia links like https://de.wikipedia.org/wiki/Klammer_(Zeichen)

There is only open question where to keep the styles? Many reset code was needed hence nextcloud's styles overwrites some default HTML styles globally.

For the component itself I'd actually prefer to keep them renderless. However we could of course just export some common markdown styling so that apps could embed that then if needed.

Cacodaimon commented 4 years ago

That would probably something worth to check if we can make use of the same libs in text as well, since implementing a parser with the markdown-it capabilities also seemed like a a lot of custom implementation effort, so we are using some dirty workarounds over there right now.

Remind me about this when this issue is solved, maybe I can help out.

I personally prefer that behaviour however the main concern @nickvergessen had about this is that this breaks for example wikipedia links like https://de.wikipedia.org/wiki/Klammer_(Zeichen)

I see maybe opening a ticket here solves the issue (somewhere in the future) without putting own effort about fixing this, with the current auto link implementation like like this one http://git (local office git server) or https://192.168.1.101/foo does not get detected. So maybe is failing test can be ignored?

For the component itself I'd actually prefer to keep them renderless. However we could of course just export some common markdown styling so that apps could embed that then if needed.

Currently all classes can be overwritten but I could although provide the default styles as a SASS + CSS file in the package dist folder, for self including, and omit the VUE component's styles.

tromlet commented 4 years ago

Holy cats that is all great work @Cacodaimon! Hopefully you and the Nextcloud guys can come to some kind of agreement!

I DID notice a couple things in your example screenshots:

  1. Text that was both bold and italicized did not appear to work with asterisks
  2. Your ordered list featured bullets, not numbers or letters identifying list items (and thus was not "ordered")
  3. Your ordered and un-ordered sub-lists were not sub-lists at all

I freaking LOVE LOVE LOVE the small table in there, and was there single line and multi-line code formatting? E.g. this and

this?
Cacodaimon commented 4 years ago

@juliushaertl You might find find a pull request once I was able to fix the rollup.config.js.

@tromlet Thanks! But the most work comes form the libs used.

I have used this markdown example as test content, maybe the lib and the test content does not use the same markdown flavor. Currently ordered lists gets rendered as unordered 🤦. But sub lists are working it seems that my test content had some formatting issues here.

Cacodaimon commented 4 years ago

@juliushaertl you should already haven been notified by github about my PR, hopefully it is fine.

So the current diff would look like: https://github.com/nextcloud/spreed/compare/master...Cacodaimon:master

@nickvergessen @juliushaertl From now on I could need some directions how to strip markdown in the PHP code for clients which does not support markdown (e.g. mobile clients)

nickvergessen commented 4 years ago

If you don't mind, drop me an email to <my github name>@nextcloud.com then i will create you an account on our instance so we can chat about it or have a call.

Cacodaimon commented 4 years ago

@nickvergessen thx I wrote you an mail about the invite.

alexbijma commented 3 years ago

@nickvergessen could you give an update on this ticket? has there been any progress or any idea when this will be available within Talk via webbrowser?

nickvergessen commented 3 years ago

It will only be available once we have time to implement it on all clients

MaxHillebrand commented 3 years ago

full markdown support would be optimal, I would use it as soon as it is deployed!

IanChok commented 2 years ago

This is a really nice feature to have. What's the status so far?

mortee commented 2 years ago

Yeah, what's up with this?

oneWaveAdrian commented 2 years ago

@nickvergessen This feature is still requested, with a lot of interest to make Talk more coherent with platforms such as Slack or Mattermost, where basic text formatting (especially code) is standard practice. What happened to that MR/PR from @Cacodaimon? Already looked very promising!

nickvergessen commented 2 years ago

Yeah it is, like many other features, but it is currently not a priority item.

The PR has been merged IIRC, the problem is that Talk/Spreed is currently not using this input method and the transition to it was not straight forward last time a colleague tried it.