feat: Apply markdowns to text messages #1120
Conversation
✅ Deploy Preview for sendbird-uikit-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| const text = value[0]; | ||
| const start = value.index ?? 0; | ||
| const end = start + text.length; | ||
| return { text, start, end, groups: value.filter((val: any) => typeof val === 'string') }; |
There was a problem hiding this comment.
요거 filter 안해주면 아주 이상한 data structure 가 나오는데 요게 any[] 로 취급이 안되서 필터를 해줘야 합니다.
이상한 data struture (array 에 key value 가 들어감):
[
'[here](https://example.com)',
'here',
'https://example.com',
index: 112,
input: '! This is a test for **tokenizeMessage**. Followings are urls with different syntaxes: https://example.com, and [here](https://example.com).',
groups: undefined
]
| groups, | ||
| }; | ||
| newTokens.push(mid); | ||
| restText = rawStr.slice(end); |
There was a problem hiding this comment.
Fixed:
past: restText = restText.slice(end);
now: restText = rawStr.slice(end);
There was a problem hiding this comment.
이 컴포넌트는 현재 이 PR 에서 사용되지 않는것 같은데, 변경 안하는게 낫지 않을까요?
There was a problem hiding this comment.
음 버그 였던것 같은데 다시 확인해서 아니면 돌려놓겠습니다.
|
@bang9 요거 알고리듬을 바꿔야 될 것 같습니다. 현재 token 으로 나눠서 하기 때문에 하나의 토큰 안에 한가지만 적용하다는 limitation 이 있기 때문입니다. token 으로 나누지 않고 오리지날 스트링에 range mapping 을 사용하면 해결 될것 같습니다. from x to y is url, from a to b is markdown bold, ... etc. |
네 변경되면 리뷰요청 주세요 |
|
@bang9 can you test again? |
|
@liamcho could you check the failed test? And it is good to add test for the nested cases when you have room! |
|
@bang9 can you review again please~ |
AhyoungRyu
left a comment
AhyoungRyu
left a comment
There was a problem hiding this comment.
Left some minor comments but not blockers. LGTM
| @@ -0,0 +1,18 @@ | |||
| export function asSafeURL(url: string) { | |||
There was a problem hiding this comment.
Can we have a comment about this util function? Brief input & output example would be helpful :)
| } | ||
| const rawStr = token.value; | ||
| // @ts-ignore | ||
| const matches = [...rawStr.matchAll(MarkdownRegex)]; |
There was a problem hiding this comment.
Do we have a reason for copying rawStr.matchAll(MarkdownRegex) and create a new array here?
There was a problem hiding this comment.
purpose was not to copy but to create an array from iterator
| const allMatches = matches.map((value) => { | ||
| const text = value[0]; | ||
| const start = value.index ?? 0; |
There was a problem hiding this comment.
| const allMatches = matches.map((value) => { | |
| const text = value[0]; | |
| const start = value.index ?? 0; | |
| const allMatches = matches.map((value, start) => { | |
| const text = value[0]; |
We can get the index directly like in this way too.
There was a problem hiding this comment.
are you sure its the same? matches.map((value, start) => { start here would be array index but const start = value.index ?? 0; is meant to get start index from match object.
Fixes: AC-2156
Changelogs
enableMarkdownForUserMessagetoUIKitOptions. When enabled, user messages that include markdowns syntaxes are now being applied to the original textSide note
In future we need to apply the rest of markdowns:
https://www.markdownguide.org/cheat-sheet/
Also we have issue with nested markdown case:Handled!case 1:**text[google](www.google.com)**=> only url is appliedcase 2:text[**google**](www.google.com)=> only url is appliedIn this PR, i will replace markdown logic with marking logic to cover above case.
After