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

Add individual unread counts #1262

Merged
merged 6 commits into from
Apr 12, 2024

Conversation

micahmo
Copy link
Member

@micahmo micahmo commented Apr 1, 2024

Pull Request Description

This PR adds specific unread count badges for each type of inbox message. This is very helpful to break down the overall unread count and quickly see where your messages are. For example, if you have mentions but no replies, you can quickly see that.

Review without whitespace.

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

qemu-system-x86_64_z3KrmDJ2oD.mp4

Checklist

  • Did you update CHANGELOG.md?
  • Did you use localized strings where applicable?
  • Did you add semanticLabels where applicable for accessibility?

Copy link
Member

@hjiangsu hjiangsu left a comment

Choose a reason for hiding this comment

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

The code LGTM! I do wonder if this is following MD3 guidelines though - looking at the MD3 guideline for badges, I don't see an example where its being used alongside a Chip. If you don't mind, could we try a couple different variations to see which one feels most like MD3?

It seems like the main placements for badges are on icons, or at the trailing edge of a larger component

image

image

Reference: https://m3.material.io/components/badges/guidelines

@micahmo
Copy link
Member Author

micahmo commented Apr 2, 2024

Thanks for the comments! I agree that this doesn't fit into MD3's examples perfectly, but obviously they don't cover every case, and I think having something like this is super helpful.

I guess the last example is most similar, so I tried putting the count directly in line with the text. What do you think?

image

@hjiangsu
Copy link
Member

hjiangsu commented Apr 3, 2024

Hmm, maybe I can pull in @machinaeZER0 for some feedback! I do think the second one seems to align a bit more with MD3 but would like to hear other opinions.

@machinaeZER0
Copy link
Collaborator

Heyyyyyy. This is cool and I like the idea! I think that the initial mockup, while not necessarily adherent to MD3 guidelines, is more scalable with larger numbers - in the second mockup, we'd start getting pretty cramped in the (albeit hypothetical) example where someone has double/triple digit numbers across the three pills at the top there. Figure we'd probably cap at a "99+" (capping at 9+ doesn't feel helpful to a user here), but I think the point still stands.

Definitely happy with the placement of the one in the bottom row, and the initial mockup feels similar enough that it feels natural on this screen, even though they're technically different implementations :) Honestly, my only real critique of the initial video is that I might nudge the dots down a pixel or two, as they feel a smidge high up on the corner of the pills to me.

Out of curiosity, is it possible to show what all the numbered dots would look like when they go into double digits?

@machinaeZER0
Copy link
Collaborator

318848749-14f4b24b-d219-483c-985c-11869d043762

^ also this example is so odd! Feels like it obscures the icons in a major way. I think for our bottom bar we've worked around this with our inbox icon being large enough to "support" a numbered dot. Is that from a specific app?

@micahmo
Copy link
Member Author

micahmo commented Apr 4, 2024

Thanks for the feedback, as always, @machinaeZER0!!

Figure we'd probably cap at a "99+"

Yes, regardless of which UI approach we take, I did add the cap at 99+.

is it possible to show what all the numbered dots would look like when they go into double digits?

Of course! I'm actually glad you suggested that because I found that in the original position, a sufficiently large badge would overlap the other buttons, so I needed to add a horizontal adjustment based on the length of the number. Here are both designs with a 99+.

image image

I might nudge the dots down a pixel or two

Sure, here's that too!

image image

Let me know what you think!

@machinaeZER0
Copy link
Collaborator

Thanks for taking the time to mock those up! Having the numbers directly next to the text fits better than I thought it would, actually. Does this section respect system font sizing? If so, that might be a reason to stick to the corner dot approach - imagining that three-pill section would get super wonky if the text elements got any bigger.

Overall I think that the corner dot is the most appropriate method presented so far, but that doesn't necessarily mean it's the best choice - just my two cents! And I think I prefer your original mockup to the version with the slightly lower dots (but again, thank you for helping me see what it would look like).

That said, my main thinking against numbers and text side by side is lack of space, and one thing does come to mind here - personally, I think the fact that the activated pill is an "active color" is enough of an navigation indicator without needing the in-pill check mark next to the active category (which also might read as confusing to users seeing the same icon used on the page for a "mark as read" action)... if we felt it was okay to axe the check, we'd gain back quite a bit of horizontal real estate! At that point, I think I'd be much more on-board with the side by side approach. What do you both think? If the goal is to try and stick to MD3 guidelines, I think that would be my suggestion :)

@micahmo
Copy link
Member Author

micahmo commented Apr 4, 2024

If I can summarize your comment, I think this is what you're leaning towards?

image

Also keep in mind two more things:

  1. My emulator has a low screen resolution, so in reality we should have more room than it looks like.
  2. Yes, this section follows the system font scaling. But also note that the buttons will wrap to a new row if needed, so again, we're not as cramped for space as you might think.

I think all of these things favor the inline approach.

@hjiangsu
Copy link
Member

hjiangsu commented Apr 4, 2024

Thanks so much for the feedback @machinaeZER0, you bring up some really good points!

I think for our bottom bar we've worked around this with our inbox icon being large enough to "support" a numbered dot. Is that from a specific app?

Funnily enough, this image comes from the official MD3 guidelines themselves 😅

if we felt it was okay to axe the check, we'd gain back quite a bit of horizontal real estate! At that point, I think I'd be much more on-board with the side by side approach

I'm on board with this as well, I agree that we don't necessarily need to have the active icon indicator. As @machinaeZER0 mentioned previously, we don't necessarily need to stick to MD3 guidelines all the time! I just thought it'd be good to experiment with different designs to see which one we prefer the most.

I'm personally okay with either choice, but if I were to have to choose one, I'd go with the inline one! I'll let @micahmo make the final decision 😄

@micahmo
Copy link
Member Author

micahmo commented Apr 4, 2024

I'll let @micahmo make the final decision 😄

No pressure! 😆 Sounds like we're happy with the inline count and no checkmark. If that's the case, this PR should be good to go whenever!

Copy link
Member

@hjiangsu hjiangsu left a comment

Choose a reason for hiding this comment

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

Just one small comment. Otherwise, it looks good!

@hjiangsu hjiangsu merged commit 6f8474f into thunder-app:develop Apr 12, 2024
1 check passed
@micahmo micahmo deleted the feature/more-unread-counts branch April 14, 2024 21:06
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