-
Notifications
You must be signed in to change notification settings - Fork 2k
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
New profile feed header #7056
New profile feed header #7056
Conversation
This reverts commit f58a7fa.
|
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.
A couple of tiny suggestions 👀
<View | ||
style={[a.flex_row, a.align_center, a.gap_sm, a.justify_between]}> | ||
<Text style={[a.italic, t.atoms.text_contrast_medium]}> | ||
Something wrong? Let us know. |
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.
Something wrong? Let us know. | |
<Trans>Something wrong? Let us know.</Trans> |
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.
Also, just wondering, will the text be able to wrap onto another line here at larger text sizes and in non-English localizations?
<Text | ||
style={[a.text_sm, a.leading_tight, t.atoms.text_contrast_medium]} | ||
numberOfLines={1}> | ||
<Trans>By</Trans>{' '} |
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.
Also, I'm wondering if it might be desirable (or indeed feasible) to restructure this, so that By
and the handle are both in the string to be translated, for languages where some flexibility is needed? 🤔
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.
Yes, Trans
should be pulled outside here
Co-authored-by: surfdude29 <[email protected]>
* Init hacking * Lil baby button checkpoint * Playing around * Revert "Playing around" This reverts commit f58a7fa. * Mostly there * Cleanups * Cleanup * Fix report dialog nesting * Remove transform on native * Rename header * Fix layout, overflowing FAB buttons * Remove hack * Couple of fixes * Keep Pin primary CTA (bluesky-social#7061) * Update src/screens/Profile/components/ProfileFeedHeader.tsx Co-authored-by: surfdude29 <[email protected]> * Simplify, use old string * Wrap Trans better --------- Co-authored-by: dan <[email protected]> Co-authored-by: surfdude29 <[email protected]>
Medium-sized step towards some cleaner UI for this screen. More work will be done here.
This PR aims to keep translation strings mostly the same, but some were added.