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

Fix #2309: Add badge on mobile if there is an unread message #2324

Merged
merged 4 commits into from
Oct 23, 2024

Conversation

pieer
Copy link
Collaborator

@pieer pieer commented Sep 1, 2024

No description provided.

@pieer pieer requested a review from taoeffect September 1, 2024 22:09
Copy link

cypress bot commented Sep 1, 2024

group-income    Run #3168

Run Properties:  status check passed Passed #3168  •  git commit fa3928a3fb ℹ️: Merge a97ad2cdd086df49ee9893d6a155e68d709bcf79 into 9fd5e1643cf0396ad0dfb58b6d45...
Project group-income
Branch Review task/#2309-new-design-on-mobile-chat
Run status status check passed Passed #3168
Run duration 09m 22s
Commit git commit fa3928a3fb ℹ️: Merge a97ad2cdd086df49ee9893d6a155e68d709bcf79 into 9fd5e1643cf0396ad0dfb58b6d45...
Committer Pierre Schweiger
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 10
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 111
View all changes introduced in this branch ↗︎

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Cool, sorta works, however the indicator doesn't always make sense.

For example, it appears on the dashboard when the user is mentioned in chat:

Screenshot 2024-09-06 at 2 54 45 PM

But pressing it doesn't help at all in this situation:

Screenshot 2024-09-06 at 2 55 52 PM

We need a better design here I think.

@corrideat
Copy link
Member

We need a better design here I think.

Another issue is that the badge appears to be floating around, so it's not immediately obvious that it's part of the button.

@pieer
Copy link
Collaborator Author

pieer commented Sep 23, 2024

I've updated the logic:
If we are anywhere in the app but the chat group, we show a notification in the left menu to indicate a new message.
If we are in the group chat page, we have on mobile, an indication on the right panel to indicate new message.

I've updated the icon design to have a background attached to the notification, so the badge is not floating anymore, but attached to the button.

If there is already notification for something else, the main menu show that notification. When the user open the menu he can either look at the notification but also can see that there is a new message in the chat

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Seems to be working, nice work @pieer!

@taoeffect taoeffect merged commit 1bc76eb into master Oct 23, 2024
4 checks passed
@taoeffect taoeffect deleted the task/#2309-new-design-on-mobile-chat branch October 23, 2024 19:46
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.

3 participants