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

Feat: Label indicating a reply must be sent in chatbox #1149

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

JoshKornfeld
Copy link
Contributor

@JoshKornfeld JoshKornfeld commented Nov 18, 2022

Description

Closes #1150

This PR implements a label onto chat previews for messages in which the logged in user was not the last one to write a message. Should a user not want to see the labels, they can use a slider at the top of the inbox to turn them off. A cookie will track this state for 30 days.

The label shows under the condition that the user was not the last one to send a message, or the chat has not had any messages sent in it yet.

The new label is only implemented for non mobile screens.

Interface before:
Screenshot from 2022-11-21 14-52-57

Screenshot displays new interface.

Screenshot from 2022-11-21 14-33-38

Test plan

  1. Go to inbox
  2. Send a message to yourself and see the label appear

Before landing

  1. PR has meaningful title
  2. yarn lint passes (frontend)
  3. yarn format passes (frontend)
  4. make format passes (backend)

@JoshKornfeld JoshKornfeld marked this pull request as ready for review November 21, 2022 13:40
Copy link
Contributor

@ddhanesha ddhanesha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You made a good try but I don't think we need to add a role column to user_profile table to show a label. There is a better way to show the label. You fetch user_id of the last message sent in a chat. Then compare the id of the user calling the API and user_id of the last message. If the user_id does not match then you show the message.

@JoshKornfeld
Copy link
Contributor Author

You made a good try but I don't think we need to add a role column to user_profile table to show a label. There is a better way to show the label. You fetch user_id of the last message sent in a chat. Then compare the id of the user calling the API and user_id of the last message. If the user_id does not match then you show the message.

We wanted to make this only available to certain users like Thomas, as this is a tool for community building. If this was for regular users we would need to add a way to dismiss the label for the cases in which their conversation is over for the time being and the label may become annoying to those users.

@ddhanesha
Copy link
Contributor

You made a good try but I don't think we need to add a role column to user_profile table to show a label. There is a better way to show the label. You fetch user_id of the last message sent in a chat. Then compare the id of the user calling the API and user_id of the last message. If the user_id does not match then you show the message.

We wanted to make this only available to certain users like Thomas, as this is a tool for community building. If this was for regular users we would need to add a way to dismiss the label for the cases in which their conversation is over for the time being and the label may become annoying to those users.

I don't think we established that this requirement should only be for climate connect community building. I believe this can be a very useful tool for everyone on the platform.

@JoshKornfeld
Copy link
Contributor Author

@ddhanesha I agree that this feature could be useful to regular users. I've removed the role column and instead introduced a slider which state is stored in a cookie. Hopefully, this will enable users to also use this feature, without annoying users who do not want to use it.
In the prior implementation, I had discussed with @positiveimpact what to do about hiding the label for users who didn't need it i.e. in the case a chat had some to its natural conclusion for the time being. We concluded that we could just make this available to certain users like Thomas and avoid needing the ability to turn it off, hence the introduction of the role column. I hope this is a good middle ground :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

Add a label to inbox indicating if the user was not the last one to write a message
3 participants