-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[🐴] Adjust messages list styles #3945
Conversation
Your Render PR Server URL is https://social-app-pr-3945.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cour1fu3e1ms73cgvn9g. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Load state is real nice. I made a PR with my thoughts here #3949
src/screens/Messages/List/index.tsx
Outdated
(hovered || pressed) && t.atoms.bg_contrast_25, | ||
index === 0 && a.border_t, | ||
a.border_b, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm still kinda anti-border I think, but that's just me. Not sure it adds much
src/screens/Messages/List/index.tsx
Outdated
{maxWidth: '85%'}, | ||
web([a.leading_normal, {marginTop: -4}]), | ||
]}> | ||
<Text style={[t.atoms.text, a.font_bold, {fontSize: 17}]}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the general notion of trying to match the posts for consistency's sake, but let's not start adding custom font sizes to do so. We should use what's in ALF and update there if we need to in the future to stay consistent with new UI and avoid having type that's like 12, 14, 15, 16, 17 all right next to each other.
@@ -154,12 +142,19 @@ let Header = ({ | |||
)} | |||
<View style={[a.align_center, a.gap_sm, a.flex_1]}> | |||
{profile ? ( | |||
<> | |||
<View style={[a.align_center]}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole header is getting a little cramped, we may want to consider doing a horizontal layout so it doesn't get too tall
* Adjust sizing * Consistent font size * Adjust header
# Conflicts: # src/screens/Messages/Conversation/MessagesList.tsx # src/screens/Messages/Conversation/index.tsx
Why
without whitespace
I'm a little concerned about obscuring handles in DMs behind long display names. Since it's a more personal conversation, I think there's benefit in making sure that the user can actually see who it is that they are talking to.
Open to ideas on this of course!
Also removes the extra spinner states that were jumpy, and prevents the list from flickering on initial render on native.
Chat List
Avatar Alignment
Use the same size and (almost) alignment as feeds so there isn't a noticeable shift. I say almost because we really need to move away from the odd-number old style system - or convert the new one to odd numbers - so there's an ever-so-slight difference here. (Note, spacing between handle and message is a little higher here, but the image above is what you should go off of. Made a change)
In the feed
In the chat
Chat Header
Chat Loading
RocketSim_Recording_iPhone_15_Pro_6.1_2024-05-09_23.46.55.mp4