Details
Tested later in the stack.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/mention-utils.js | ||
|---|---|---|
| 67 ↗ | (On Diff #30687) | Why the beginning of the line? |
| 97 ↗ | (On Diff #30687) | I don't think this is necessary. Read this. Since strings are immutable primitives, you cannot change them. You always create a new string and just change what your variable points at. |
| 106 ↗ | (On Diff #30687) | What is ET` ${thread} `? What type is it even? what.... |
| 107–109 ↗ | (On Diff #30687) | Can you explain what this does? |
| lib/shared/mention-utils.js | ||
|---|---|---|
| 67 ↗ | (On Diff #30687) | |
| 106 ↗ | (On Diff #30687) | // ET is a JS tag function used in template literals, eg. ET`something` // It allows you to compose raw text and "entities" together taken from entity-text.js. It's just an utility which can render threads uiName without using a hook. It also has other features which I encourage you to explore in entity-text.js file. |
| 107–109 ↗ | (On Diff #30687) | Replace raw chat mention with current thread name when user "sees" the thread, otherwise render alternative/default text. |
We shouldn't use entityTextToRawString directly. I'm not sure what the alternatives are here, and what the added complexity will be. Please investigate and get back to us
| lib/shared/mention-utils.js | ||
|---|---|---|
| 106 ↗ | (On Diff #30687) | entityTextToRawString does not handle ENS resolution, and as such we can't use it here. This is going to add a ton of complexity unfortunately. You'll need to either use useEntityTextAsString (hook) or getEntityTextAsString (async function). If you opt for a hook, it's probably easier to use useResolvedThreadInfo. |
Thought about this some more... I suspect you haven't given much thought to ENS names, and to properly handle it will need updates across the whole stack, from the search index, to the display of results, to the representation of the "markdown", to the final rendering of the tag.
If it's true that you haven't given much thought to it, it would probably be best to handle this as part of ENG-2199. Can you add a comment there? Please make sure to link any prior discussion (such as this diff, and anywhere else ENS names or EntityText have been discussed for your project).
I'm no longer using entityTextToRawString since I only need to extract alternative text. Please take a look at the latest diff, 31258. Throughout the entire stack, I use the useResolvedThreadInfo hook. Also, I'm not handling ENS names in this stack; user mentions are handled as they were before. The only change was made in the search index -> however this change does affect mentioning in the input bar only. I can add a comment that the new search index needs to be refactored to support ENS names.
Oops, sorry!! Thanks for clarifying, that was my bad.
Also, I'm not handling ENS names in this stack; user mentions are handled as they were before.
ENS names don't only appear to user mentions. They also appear in chat mentions. If you have a nameless chat with two friends and one of them is an ENS user, then the uiName includes both of those users' usernames, and as such it needs to be ENS-resolved.
I can add a comment that the new search index needs to be refactored to support ENS names.
That would be appreciated! It would be helpful if you could share code pointers to places that will need to be updated in order for the chat-mentioning code to support ENS resolution.