Enhance notification feed item to display more (3) author names #962
Conversation
| {formatCount(authors.length - 1)}{' '} | ||
| {pluralize(authors.length - 1, 'other')} | ||
| {listFormatNodes( | ||
| new Intl.ListFormat('en', {style: 'long', type: 'conjunction'}), |
There was a problem hiding this comment.
This is giving me an error in iOS and Android; I'm pretty sure the RN runtime doesn't support it yet
There was a problem hiding this comment.
Ah. I see. That is why I had to slap the triple-slash on the top of the file to get TypeScript to be quiet.
<sigh> .. I would suggest my first version of this (it worked), but it was pretty complex.
Would you be opposed to a poly-fill perhaps? (https://formatjs.io/docs/polyfills/intl-listformat/)
There was a problem hiding this comment.
Well I would, but see my other comment. I'm not sure I'm on board with the PR's intent unfortunately!
|
I gotta be honest I find this pretty visually difficult to parse 
I feel like the extra tap to expand the avatars is pretty low effort and keeps things visually easier to scan |
|
@pfrazee I am inclined to agree in the case of a large number of authors. But I would say that it probably excels (visually) in cases where there is only ~2 authors in the notification. But.. my UX chops are limited so I cannot think of a great way to decide when that should be shown versus not. (Only inline if there are 2 or less authors?) I was also just agreeing with some folks who were discussing that only showing one name was slightly annoying to them: https://bsky.app/profile/danabramov.bsky.social/post/3jznlndi7ot2z Any suggestions for alternatives? If not I don't mind if this get's improved in some future day. (The attempt was still fun, this is a good codebase for it's early age!) Thanks for having a look! |
|
Oh okay -- I think for that we just special case the "2 people" situation. That should probably take a different approach than this. Thank you though! |
|
Definitely different approach. Thanks for taking the time to leave feedback @pfrazee! |
Hi! 👋
The notification's page is a little too brief with displaying the "who" part of a notification, so I thought I would increase it to about 3. The code handles the constant being updated very well, so if it needs to be changed it shouldn't be an issue.
I have tested this by running a local expo frontend just attached to my production account and I looked at my own notifications. My data in production seems to cover all the cases I've considered in my solution.
I think I followed most of the written contributor guidelines, but sorry if I missed something important.
Let me know if I missed anything or if you have any questions! Thanks!