-
-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#2427 - Add 'reply' menu for a proposal notification message. #2491
#2427 - Add 'reply' menu for a proposal notification message. #2491
Conversation
group-income
|
Project |
group-income
|
Branch Review |
sebin/task/#2427-add-reply-option-to-automated-message
|
Run status |
|
Run duration | 10m 41s |
Commit |
|
Committer | Sebin Song |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
10
|
|
0
|
|
112
|
View all changes introduced in this branch ↗︎ |
const proposal = message.proposal | ||
const getNameFromMemberID = (memberID) => { | ||
const profile = this.globalProfile(memberID) | ||
return profile ? `${CHATROOM_MEMBER_MENTION_SPECIAL_CHAR}${memberID}` : L('Unknown user') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this check needed? I believe ${CHATROOM_MEMBER_MENTION_SPECIAL_CHAR}${memberID}
should be used unconditionally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
(I thought the user profile data of a member who has left group would be gone and no longer captured in ourContactProfilesById
getter. But it seems that's not the case which is a good thing.)
@taoeffect Just updated the PR again. BTW, it's a minor suggestion but while working on the update, I noticed the avatar image of a member who left the group is just a grey background. ![]() That looks fine but what's your thoughts on showing this question mark icon on top of it too? ![]() If you think it's a good idea, pls create an issue and assign me to it. |
I think it's a fine idea, but I feel neutral to it. If you feel strongly that you'd like to see this, feel free to create the issue and self-assign. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @SebinSong!
closes #2427
I have worked on this issue for
MessageInteractive.vue
component, which is used for various proposal-related notifications in the chat .[Screenshot - Menu added]
[Screenshot - When the 'reply' menu is clicked]
[Screenshot - replied message]