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: Display 'Unread Message' Divider Functionality Similar to RC #766

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

dhairyashiil
Copy link
Contributor

Brief Title

Acceptance Criteria fulfillment

  • If the user is scrolled up and someone sends a message, after clicking the 'New Messages' popup, the 'unread message divider' will appear above the unread messages.
  • If there are multiple unread messages, the 'unread message divider' will only show above the first unread message.
  • After any activity from the user, such as replying to the message or reopening the chat, the 'unread message divider' will disappear.

Fixes #765

Video/Screenshots:

1.mp4

and

2.mp4

PR Test Details

Note: The PR will be ready for live testing at https://rocketchat.github.io/EmbeddedChat/pulls/pr-766 after approval. Contributors are requested to replace <pr_number> with the actual PR number.

@abirc8010
Copy link
Contributor

Hi @dhairyashiil , could you modify it with the colors in theme object , so that it looks consistent across different themes ?

@Spiral-Memory
Copy link
Collaborator

Hey @dhairyashiil
Thanks for this, I really liked it

Copy link
Collaborator

@Spiral-Memory Spiral-Memory left a comment

Choose a reason for hiding this comment

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

LGTM !

@Spiral-Memory
Copy link
Collaborator

Hi @dhairyashiil , could you modify it with the colors in theme object , so that it looks consistent across different themes ?

@dhairyashiil pls incorporate this

@dhairyashiil
Copy link
Contributor Author

Hey @dhairyashiil Thanks for this, I really liked it

Thank you, @Spiral-Memory. Hearing this from you is quite a compliment. Indeed, a Happy New Year!

@dhairyashiil
Copy link
Contributor Author

Hi @dhairyashiil , could you modify it with the colors in theme object , so that it looks consistent across different themes ?

Hello Abir,

Right now, RC has 3 themes:

  1. Light
  2. Dark
  3. High Contrast
    In all 3 of them, the same red color has been used.

Regarding the Embedded Chat, this feature was not present earlier, so now we need to decide which color should be set from the theme. Although I think this red color is suitable for all the themes we currently have, I believe that the 'unread message' should feature an eye-catching color that’s easy to notice. I think the current red handles this well.

Abir and @Spiral-Memory, if you still think we should change it according to the theme, please let me know which color to select.

@abirc8010
Copy link
Contributor

In all 3 of them, the same red color has been used.

Hi @dhairyashiil, unlike Rocket Chat, EmbeddedChat is not a standalone chat application. Its primary purpose is to be embedded within any website, allowing external developers to configure and customize it to their preferences.

For development purposes, it is presented inside Storybook. You can experiment with passing a theme object through the controls to define your designs. Additionally, you can explore the pre-built themes in the design variants. Instead of hardcoding colors, using colors from the theme object provides external developers with greater flexibility and control over customization.

Abir and @Spiral-Memory, if you still think we should change it according to the theme, please let me know which color to select.

Maybe you can use theme.colors.destructive or explore other colors in DefaultTheme.js and use lighten or darken to find a suitable shade in default mode and pre defined themes in storybook.

@dhairyashiil
Copy link
Contributor Author

In all 3 of them, the same red color has been used.

Hi @dhairyashiil, unlike Rocket Chat, EmbeddedChat is not a standalone chat application. Its primary purpose is to be embedded within any website, allowing external developers to configure and customize it to their preferences.

For development purposes, it is presented inside Storybook. You can experiment with passing a theme object through the controls to define your designs. Additionally, you can explore the pre-built themes in the design variants. Instead of hardcoding colors, using colors from the theme object provides external developers with greater flexibility and control over customization.

Abir and @Spiral-Memory, if you still think we should change it according to the theme, please let me know which color to select.

Maybe you can use theme.colors.destructive or explore other colors in DefaultTheme.js and use lighten or darken to find a suitable shade in default mode and pre defined themes in storybook.

Thanks for the clarification! I understand that EmbeddedChat is for embedding in websites, not a standalone app like Rocket Chat. I'll explore the theme.colors.destructive, other colors in DefaultTheme.js 👍

@dhairyashiil
Copy link
Contributor Author

Hello @abirc8010,

I tried setting the color from "destructive," but it didn't look good with the dark theme.

Later, I explored all the options in EmbeddedChat/packages/react/src/theme/, and after that,
I believe the most suitable color for the unread message divider should be 'theme.colors.foreground' as the foreground color will always be the opposite of the background color.

I checked it for both light and dark theme variants, and it looks good.

@Spiral-Memory
Copy link
Collaborator

In theme object, there is another key, commonColors or something similar, you can have the required color there.. if that color looks good in all themes provided..

@dhairyashiil
Copy link
Contributor Author

Hello @Spiral-Memory,
I checked it, and the common colors have only two options across all the different variants: black and white.

Please refer to this image:
image

I believe that instead of setting it to black or white, setting it to "foreground" is a better option, considering all the different variants and their dark and light themes.

Below, I have attached a video where I demonstrate this across different variants and their dark and light themes:

unread.message.for.all.variants.in.light.and.dark.theme.2.mp4

@Spiral-Memory
Copy link
Collaborator

Spiral-Memory commented Jan 5, 2025

In commonColors, you please add your color as well, if your red color is consistent among all themes

@Spiral-Memory
Copy link
Collaborator

Or go with an error color..

@dhairyashiil
Copy link
Contributor Author

@Spiral-Memory , Okay, how about this:

For all light themes, we will use the 'destructive' color, which is red or sometimes dark red.
For all dark themes, we will use the 'warningForeground' color, which is an orangish color.

Refer:
image

and

image

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

Successfully merging this pull request may close these issues.

Feat: Display 'unread message' functionality Similar to RC
3 participants