[PAY-168] Redesign rest of notifications#1283
Conversation
|
Preview this change https://demo.audius.co/dj-pay-168-redesign-rest-of-notifications |
rickyrombo
left a comment
There was a problem hiding this comment.
These look so pretty!! 😍
| {otherUsersCount > 0 ? messages.others(otherUsersCount) : null} | ||
| {messages.reposted} | ||
| {entityType.toLowerCase()} |
There was a problem hiding this comment.
nit: These all seem like part of one "message", can we have some method or something return a string with all of these templated together for clarity?
There was a problem hiding this comment.
yeah for sure! i was trying something out by putting it all in as react components, you think it's confusing ultimately? i guess it's hard to know that that is literally all just text huh? where it get's a lil tricky is when there is text that is also a link + tooltip, then i couldn't really template that in. thoughts?
There was a problem hiding this comment.
Yeah that's true... Generally I think having it all be one "message" makes it easier in the future for other translations to make it grammatically correct, otherwise the grammar is already baked in by the ordering of the rendered strings.
Just looked up https://react.i18next.com/latest/trans-component and maybe it's a non issue? Feel free to ignore for now and we can revisit when doing i18n
| @@ -1,5 +1,5 @@ | |||
| .root { | |||
| margin-right: 4px; | |||
| .rootLeftMargin { | |||
There was a problem hiding this comment.
to support RTL, should this be margin-inline-start?
There was a problem hiding this comment.
ohh that is genius great catch
| <NotificationBody> | ||
| <EntityLink entity={entity} entityType={entityType} /> | ||
| <span>{messages.by}</span> | ||
| <UserNameLink user={user} notification={notification} leftMargin /> |
There was a problem hiding this comment.
Feels like the previous span should have a margin-inline-end vs having a prop for left margin in UserNameLink?
There was a problem hiding this comment.
completely agree, was wigging out on how to do this, styling def aint my strongsuit
Description
Redesigns the rest of the notifications