Skip to content
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

Design for Pinned Messages in Chat #1963

Closed
taoeffect opened this issue Apr 22, 2024 · 14 comments · Fixed by #2011
Closed

Design for Pinned Messages in Chat #1963

taoeffect opened this issue Apr 22, 2024 · 14 comments · Fixed by #2011
Assignees
Labels
App:Frontend Kind:Enhancement Improvements, new features, performance upgrades, etc. Note:UI/UX Priority:High

Comments

@taoeffect
Copy link
Member

Problem

We need to be able to pin important messages in chatrooms as well as in DMs.

Solution

Create a design, similar to the below, (and later on, implement it).

Screenshot 2024-04-22 at 10 01 01 AM

The "Pinned" menu should appear only if there are pinned messages.

A "Pin message to chatroom" menu item should be added to in the dropdown menu for every message.

@taoeffect taoeffect added Kind:Enhancement Improvements, new features, performance upgrades, etc. Note:Up-for-grabs App:Frontend Priority:High Note:UI/UX labels Apr 22, 2024
@Silver-IT Silver-IT self-assigned this Apr 29, 2024
@Silver-IT
Copy link
Member

Silver-IT commented May 17, 2024

@taoeffect, I can see that Font Awesome doesn't provide the pin icon.
Should we store the pin image in png format, or use the Font Awesome paperclip instead of pin icon?
image

@taoeffect
Copy link
Member Author

@Silver-IT I used the emoji 📌 for that. Feel free to either use the emoji or update our font-awesome version, or maybe check if ours already has the "thumbtack"

@Silver-IT
Copy link
Member

Oh, yeah. We have thumbtack. I think we should use that. Thanks.

@Silver-IT
Copy link
Member

In order to fully implement this issue, the PR #2005 should be merged.

@Silver-IT
Copy link
Member

@taoeffect, we have four types of messages.

export const MESSAGE_TYPES = {
  POLL: 'poll',
  TEXT: 'text',
  INTERACTIVE: 'interactive',
  NOTIFICATION: 'notification'
}

Do you think we should make all of these four types to be pinned? In my opinion, I would like to allow only one type of message (MESSAGE_TYPES.TEXT). I think only text messages can be pinned in Slack.

@taoeffect
Copy link
Member Author

@Silver-IT is there some technical reason why pinning any of them would be hard?

What do we use INTERACTIVE for? 🤔

I think we should at least let polls and text messages be pinned. Notifications (e.g. "so and so joined the group") don't need to be pinnable, but also it wouldn't really hurt anything if you could pin such messages.

@Silver-IT
Copy link
Member

@Silver-IT is there some technical reason why pinning any of them would be hard?

Of course, I don't think there is a technical blockers. We've already implemented all of them in the ChatMain.vue. I just wanted to simplify as much as possible.

What do we use INTERACTIVE for?

Actually, MESSAGE_TYPES.INTERACTIVE is what @SebinSong used. I will check the Figma design again and the necessary of that message type.

I think we should at least let polls and text messages be pinned. Notifications (e.g. "so and so joined the group") don't need to be pinnable, but also it wouldn't really hurt anything if you could pin such messages.

If you think we should do, I will do that way.

@SebinSong
Copy link
Collaborator

SebinSong commented May 23, 2024

@Silver-IT Hey man, what exactly did you intend to say by this? I don't remember creating this message type by myself.
cc. @taoeffect

Actually, MESSAGE_TYPES.INTERACTIVE is what @SebinSong used.

@Silver-IT
Copy link
Member

Silver-IT commented May 23, 2024

@SebinSong, just wanted to say you would be better to answer his question: 'What do we use INTERACTIVE for?'. I think that message type is only being used inside gi.actions/group/notifyExpiringProposals function which you created long time ago.

cc: @taoeffect

@Silver-IT
Copy link
Member

@taoeffect, I have one question too.
Should we make it possible to display pinnedMessages also for the chatrooms which the current user is not part of?

The general solution to display pinnedMessages for those chatrooms is for users to to sync those chatrooms in order to get the list of pinned messages. Another tricky solution could be save the pinned messages in the group state, but it won't be answer for the public chatrooms which the users are not part of those chatrooms and their groups too.

So my opinion is to skip displaying pinned messages for chatrooms which the users are not part of.

@SebinSong
Copy link
Collaborator

@Silver-IT , @taoeffect
I did not create that action either, man..
but did touch that part as part of the work for notifying an expiring proposal a while ago.
It seemed to me that the original author specified interactive type there because it contains a call-to-action in it(in that case, I think it was a link tag navigating to the proposal widget).

@Silver-IT
Copy link
Member

Okay, then, Sorry, if you don't use that message type. Anyway, I was planning to check Figma and message types too.

Thanks for clarification, @SebinSong.

cc: @taoeffect

@SebinSong
Copy link
Collaborator

No problem, @Silver-IT . Thanks.

@taoeffect
Copy link
Member Author

@Silver-IT

Should we make it possible to display pinnedMessages also for the chatrooms which the current user is not part of?

Nah.

So my opinion is to skip displaying pinned messages for chatrooms which the users are not part of.

Agreed 👍

@Silver-IT Silver-IT linked a pull request May 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App:Frontend Kind:Enhancement Improvements, new features, performance upgrades, etc. Note:UI/UX Priority:High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants