-
Notifications
You must be signed in to change notification settings - Fork 984
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
Change ui of chat message #9757
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (32)
|
@@ -93,6 +93,10 @@ | |||
{:align-self :flex-start | |||
:padding-left (if platform/desktop? 24 8)})) | |||
|
|||
(def message-author-touchable | |||
{:margin-left 12 | |||
:padding-horizontal 2}) |
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.
@Ferossgp hey, my mistake. it should be vertical here if needed at all (text line height is 18, touchable is 22 so 2px padding would center it inside the touchable)
776ccb7
to
14c60bc
Compare
{:font-size 40 | ||
:desktop {:line-height 46} | ||
:margin-top (if incoming-group 4 0)}) | ||
{:font-size 40 |
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.
is that emoji only font-size? 40 is hardcore, can we lower it to 28
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, it is only emoji size - I can do it.
14c60bc
to
803df43
Compare
97% of end-end tests have passed
Failed tests (3)Click to expand
Passed tests (97)Click to expand |
8c448bb
to
ddfc59b
Compare
• Rounded corners ✅ |
9a21f1e
to
75a0b0d
Compare
Reply icons are correct now! ✅ Areas of the #9469 issue which are not fixed yet: • 3. Make sure the minimum space in a line between a timestamp is 12 (is 6 now, I kinda like it but it's probably wiser to have a larger margin) |
bd7693a
to
4d4b4d7
Compare
Message author values are correct ✅ left in the issue, can be divided into separate PRs. |
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.
do we really need all these fonts? and BETA fonts ?
(when justify-timestamp? {:position :absolute | ||
:bottom 7 | ||
:bottom (+ 6 3) ; 6 Bubble bottom, 3 message baseline |
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.
why don't just use 9
? and left (+ 6 3) in comment?
@flexsurfer I haven't looked if they are used, I updated only the fonts which were already present. I would suggest to cleanup them in a sperate PR in case it may affect some platforms or code. |
46586dc
to
b362e7d
Compare
@flexsurfer that's a great question. to be honest, we don't. we use the following font weights: BETA, they're not really 'beta' like an app :) for those open source fonts, it means that some contributors haven't provided languages like Cyrillic, Farsi etc They're safe to use, but yea, we don't use them yet in the app. |
91% of end-end tests have passed
Failed tests (9)Click to expand
Passed tests (91)Click to expand |
44% of end-end tests have passed
Failed tests (5)Click to expand
Passed tests (4)Click to expand
|
60% of end-end tests have passed
Failed tests (2)Click to expand
Passed tests (3)Click to expand
|
Tested with iOS and Android (v8.1 and v9), ready for merge! |
when merging pls don't close the original issue as there are some items left unresolved there 🙏 |
Update the rounded corners on all messages Update paddings and margin for message author Move timestamp 3pt from bottom Fix miss-placed padding in message author name Add margin between emoji and timestamp Change reply icon Decrese font size of emoji message Remove extra space in style map Remove extra margin between author and message Replace reply icon with glymph Update Inter font to support new glyphs Update paddings for reply author Update timestamp padding from bottom Added line-height explicitly to support it cross platform. Otherwise android and ios use different size. Remove margin right space on usernames Remove hardcoded width of messages Use same line-height for all types of names Add background to emoji Bug in RN emoji cropped on smaller line-height facebook/react-native#18559 Fix reply on user with ens name Fix message margin top should be always 4 Add minimal fix for ui in ens name screen Remove extra computations for timestamp position Update e2e test Signed-off-by: Gheorghe Pinzaru <feross95@gmail.com>
b2bd279
to
f34a7bc
Compare
Adds some UI changes as a part of #9469 further improvements require bigger refactor on message components.