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: simplify notifications #1905

Merged
merged 3 commits into from
Mar 19, 2023
Merged

feat: simplify notifications #1905

merged 3 commits into from
Mar 19, 2023

Conversation

antfu
Copy link
Member

@antfu antfu commented Mar 19, 2023

Before/After

image

Made based on a few assumptions:

  • People don't usually need to directly interact with the post in notification
  • The post author already has the context of the post; Having media and polls are usually redundant
  • Notifications should be concise and fix as much info as possible on one screen.
  • We don't need to duplicate our avatar and name in the notification

cc @patak-dev let me know WDYT :)

@stackblitz
Copy link

stackblitz bot commented Mar 19, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Mar 19, 2023

Deploy Preview for elk-docs canceled.

Name Link
🔨 Latest commit a81b055
🔍 Latest deploy log https://app.netlify.com/sites/elk-docs/deploys/641772ed8b1c2900085fec2e

@netlify
Copy link

netlify bot commented Mar 19, 2023

Deploy Preview for elk-zone ready!

Name Link
🔨 Latest commit a81b055
🔍 Latest deploy log https://app.netlify.com/sites/elk-zone/deploys/641772ed18dac700083d7329
😎 Deploy Preview https://deploy-preview-1905--elk-zone.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

reblog: true as any,
}],
}"
/>
Copy link
Member Author

Choose a reason for hiding this comment

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

These are more like a workaround, as I want to share the interface and avoid duplication. Maybe it's better to do it directly in the notification merging algorithm so we could always create a group with one reblog/favorites?

@patak-dev
Copy link
Member

I like it! The current grouped notifications UI looks out of place after the changes we did in the app. We had some discussions a while ago in Discord and some people were pushing for keeping the names but I think that the avatars as you did are better here (and I think most people will be happier with them).

One thing that doesn't click with me though is that it is very usual for folks to both reblog and like a post, so their avatar would usually be repeated. If you have 3 doing the same, I think it gets a bit strange. Twitter doesn't have this issue because they always separate retweets from likes. What do you think if we don't show the avatar in the likes if the same user reblogged it? And to avoid missing this information, we could add a small heart icon in the bottom-right corner of the people that reblogged+liked?

About avoiding media, polls, and expansions, that could be more controversial but I agree with you. If later on, users complain, we could add an option to have them in notifications. One issue I see though is that there are some posts that don't have any text... it is just the media. So I think it would be good to check if there is any text before hiding the media or poll, and if it is empty show it to have some reference.

There is currently a margin in the posts in notifications, did you try with no margin? So it is the same as an expanded post. I like how you did it, just curious if you played with this.

@antfu
Copy link
Member Author

antfu commented Mar 19, 2023

I feel the current "heart" after reblogging a bit confusing, especially when users have a lot of emojis after their names, making it hard to distinguish what the heart it meaning. I am actually ok with avatar duplication for like/reblog. But if you think it's an issue, I guess maybe separating them into two notifications while merging more aggressively might be a better solution (like Twitter does). As I guess the "bottom-right corner indicator" also makes it hard to understand what is going on.

No text post

Interesting, I didn't think of that. I will play with it a bit to see how it looks like.

Margin

Do you mean the left padding? Emm I think with the padding makes it more readable:

image

@patak-dev
Copy link
Member

I feel the current "heart" after reblogging a bit confusing, especially when users have a lot of emojis after their names, making it hard to distinguish what the heart it meaning. I am actually ok with avatar duplication for like/reblog. But if you think it's an issue, I guess maybe separating them into two notifications while merging more aggressively might be a better solution (like Twitter does). As I guess the "bottom-right corner indicator" also makes it hard to understand what is going on.

I kind of like having reblogs and likes as you did together, specially because in Mastodon a like is less important than in Twitter (where it has a real effect boosting your post). What if we do:

In case A1, A2, A3, A4 reblogged and liked the post, and A5, A6 only liked:

Reblog [ A1 ] [ A2 ] [ A3 ] [ A4 ] reblogged your post
Like [ A5 ] [ A6 ] and 4 others liked your post

And if you hover on "4 others" you see the list (A1, A2, A3, A4). So we avoid the duplication of avatars while keeping your current design and without introducing more noise with extra hearts.

We could also not show the likes at all of the people that reblogged. I feel like if someone boosted your post, that counts so much more than a like and it is totally redundant if they press or not the like button.

I'm fine if you would like to merge the current design with the duplicates though and we can iterate in future PRs with other ideas too 👍🏼

Do you mean the left padding? Emm I think with the padding makes it more readable:

Uh... yes, by far. Looks a lot better with the padding 👍🏼

@antfu
Copy link
Member Author

antfu commented Mar 19, 2023

The "4 others" is a great idea! Tho I am a bit lazy as I see there are a few extra things to take care about, like:

  • X reblogged
  • 1 others liked (if no one else liked as well)

I do like the simplicity of directly hiding them so I did it for now. We could see how it goes and adjust :)

@patak-dev
Copy link
Member

Looks great to me! Let's try it out

@patak-dev patak-dev merged commit 5dd3f4b into main Mar 19, 2023
@patak-dev patak-dev deleted the feat/simple-notification branch March 19, 2023 20:55
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.

2 participants