Satellite-im / Core-PWA

Satellite Core is a decentralized p2p voice, video, and text chat application and is under heavy development. Check back soon for updates, or check out the latest version at https://core-dev.satellite.im
https://core-dev.satellite.im
Other
40 stars 16 forks source link

feat(tag): highlighted user tag #5366

Closed ThomBos closed 1 year ago

ThomBos commented 2 years ago

What this PR does 📖

Which issue(s) this PR fixes 🔨

Special notes for reviewers 🗒️

Additional comments 🎤

github-actions[bot] commented 2 years ago

Download the .APK for this pull request:

ThomBos commented 2 years ago

tried many attempts, but i'm still struggling to find a way to add a function to the returned component, lmk if you know any way to accomplish such a thing 😬

josephmcg commented 2 years ago

I added click functionality to the tag messages, but we need to consider how to attach the proper information to the payload.

Probably an object with name and did would be best, then you can fetch did based on the body message, then fetch latest info from user manager

However, the message type is quickly getting bloated. Rather than appending several optional properties, we should define something most robust using generics which determine a single property. Definitely a separate issue, but I wanted to throw it out there

@molimauro what do you think?

molimauro commented 2 years ago

I was thinking the same about the payload, when i wrote the md file i had in mind a way to pass callbacks, i just need to recover that idea. I'll try to have a look at this when i have time ;)

Agree with the use of typescript generics, there are several types like the messages one that can benefit a lot! Thanks @josephmcg for the input, we can create a ticket and focus on that when we have time!

ThomBos commented 2 years ago

I think this is ready and can be tested, let me know if there's anything else to change 💥

josephmcg commented 2 years ago

Users will eventually be able to change their name, so we cant rely on the name in the message to identify someone. Plus multiple people in a group could have the same name.

Can you check the solution I suggested in a previous comment?

josephmcg commented 2 years ago

I'm working on this now, it's a bit tricky.

edit -- I hit a wall as well and reset the branch. I can take another look at this tomorrow

Was using this for inspiration https://v12.discordjs.guide/miscellaneous/parsing-mention-arguments.html#how-discord-mentions-work

josephmcg commented 2 years ago

I got sucked into a different rabbit hole today, didn't have time to look into this

josephmcg commented 2 years ago

I couldn't find a simple way to integrate this with our current setup.

https://github.com/ariabuckles/simple-markdown Our markdown library is deprecated. They claim development has moved to a new repo but there haven't been any updates to src in 6 months. We should consider something else

Slate looks super nice, but only for react. https://github.com/ianstormtaylor/slate

markdown-it seems robust enough https://github.com/markdown-it/markdown-it

I'll experiment with markdown-it tomorrow

molimauro commented 2 years ago

I couldn't find a simple way to integrate this with our current setup.

ariabuckles/simple-markdown Our markdown library is deprecated. They claim development has moved to a new repo but there haven't been any updates to src in 6 months. We should consider something else

Slate looks super nice, but only for react. ianstormtaylor/slate

markdown-it seems robust enough markdown-it/markdown-it

Slate would have been my primary choice for sure, i was considering porting it to Vue when have time (there is already a lib that ports Slate to Vue but does not seem updated). It's the lib Discord is using (kind of, a fork of Slate).

Markdown-it was our old one, Matt wanted to replace that (cant remember why), but we can discuss and see if that makes sense to integrate back.

I would maybe switch to the new simple-markdown lib, even though im not sure how to solve this problem. I think here the big problem is that we cannot render an id under the hood right?

josephmcg commented 2 years ago

I think our source and result being the same string is somewhat problematic. We can't programmatically replace content because that content will no longer continue to match the initial condition.

I suggested a new library because the current one seems dead, or at least heading in that direction. Markdown-it is more popular and also supports plugins, so there may be an out-of-the-box solution to mentions.

The discord guide was pretty enlightening https://v12.discordjs.guide/miscellaneous/parsing-mention-arguments.html#how-discord-mentions-work

molimauro commented 2 years ago

Yes exactly, our only problem is only while we type, once the message is sent we can replace easily the id with a name. We can try keeping track of when mentions are inserted/removed in the chatbar like in an array. Or another temp solution can be replace the name in the chatbar with @mauro#Lwkgh1 so that we have a ref of the did (or even visually hide the # part but this is not a lot elegant)

ThomBos commented 2 years ago

Yes exactly, our only problem is only while we type, once the message is sent we can replace easily the id with a name. We can try keeping track of when mentions are inserted/removed in the chatbar like in an array. Or another temp solution can be replace the name in the chatbar with @mauro#Lwkgh1 so that we have a ref of the did (or even visually hide the # part but this is not a lot elegant)

A thing that might work, could be keeping that # in the tag, in order to filter users using that.... I don't see any other ways even adding a "tags" field on messages, because it doesn't have any did or id to detect which user is. (so in case one user changed name we'd have the same issue)

molimauro commented 2 years ago

We can also add the # part to a data attribute

https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes

ThomBos commented 2 years ago

We can also add the # part to a data attribute

https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes

it looks interesting, need to check if the filter we have detects the html element after the refresh

ThomBos commented 2 years ago

it now checks for did or name after an @, but it cannot check for the key we are now passing to the html

ThomBos commented 2 years ago

this is the best i could think of.... might be better finding another markdown library and implement it instead of merging this code, cause it might be a bit heavy 😬 the only good thing, is that it does its job 🔨