Fix:the user name in inline comments section is followed by some internal code which should be omitted in UI [SDESK-6685] #4161
Conversation
…rnal code which should be omitted in UI [SDESK-6685]
|
Did you manage to reproduce it? If so where? I tried on sd-develop, but couldn't. |
it is reproduced on the sd-develop, I shared the snapvideo please check. https://www.awesomescreenshot.com/video/13342665?key=feb1b50cb31e7ab975577ebcba9a10ed |
|
I see. I was looking at article comments. One issue with your approach is that if there is text after the mention it is not shown. Another thing, is that mentions should be displayed like this. There is code already in article comments that is doing the extraction of user ID and name. Could you reuse it and make editor comments look the same? |
| @@ -13,7 +13,7 @@ const mentionRegexp = /@\[([^\]]+)\]\((desk|user):([^\)]+)\)/g; | |||
| * @description Displays a text containing mentions. | |||
| */ | |||
| export const TextWithMentions: React.StatelessComponent<any> = ({children, ...props}) => { | |||
| const msg = children; | |||
| const msg = props.data ? props.data : children; | |||
There was a problem hiding this comment.
can you add TypeScript types to props of this component?
Given children can only be a string and that there's only one usage of the component, it'd be better to use a standard prop like message: string. We'd then get better enforcement of types by TypeScript and it would work well with angular too without the need to accept the same data via 2 props like now.
| @@ -11,7 +11,8 @@ <h4 class="label">{{commentObj.fieldName}}</h4> | |||
| <div class="flex-row sibling-spacer-10"> | |||
| <sd-user-avatar data-user="users[item.data.authorId]"></sd-user-avatar> | |||
| <div> | |||
| <div class="text" sd-comment-text data-name="{{users[item.data.authorId].display_name || users[item.data.authorId].username}}" data-text="{{item.data.msg.split('(user:')[0]}}"></div> | |||
| <b>{{users[item.data.authorId].display_name || users[item.data.authorId].username}}:</b> | |||
There was a problem hiding this comment.
Can we modify the back-end so display_name defaults to username if not provided. Isn't it already like that @petrjasek ?
There was a problem hiding this comment.
it should be like that already
There was a problem hiding this comment.
great, then you can remove the fallback @devketanpro
|
|
||
| export interface ITextWithMention { | ||
| message: string; | ||
| props: any; |
There was a problem hiding this comment.
props isn't used anywhere, you should remove it. Also since we now have a TypeScript interface, we no longer need propTypes for this component, do remove it. Regarding naming, we have a convention to use IProps/IState for interfaces used for react component props/state - let's stick to that here too.
| export interface IProps { | ||
| message: string; | ||
| } | ||
| export const TextWithMentions: React.StatelessComponent<any> = ({message, ...props}: IProps) => { |
There was a problem hiding this comment.
propsisn't used anywhere, you should remove it
React.StatelessComponent<any> = ({message, ...props}
// ^ I meant from everywhere, not only interface
There was a problem hiding this comment.
I was about to remove it but then I thought if anyone pass other prop than message then it would be wrong.
There was a problem hiding this comment.
The interface won't allow passing any other prop than message. If someone wanted to pass anything else then they would add it everywhere - to the interface and to implementation.
There was a problem hiding this comment.
Okay, so I will remove it.
…ps instead of "any" type
No description provided.