-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix jumping composer when entering emojis or markdown text #40128
Fix jumping composer when entering emojis or markdown text #40128
Conversation
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.
Overall seems fine, let me go through everything and see if we can simplify some more 😄
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.
@fedirjh thanks for the review. This PR is changing only Composer component for web. Both of the bugs you found are also present on main.
This one is already reported, #15734
This is a feature that was implemented in #39597 |
@fedirjh bump |
Reviewer Checklist
Screenshots/VideosAndroid: NativeCleanShot.2024-04-27.at.05.25.15.mp4Android: mWeb ChromeCleanShot.2024-04-27.at.06.03.12.mp4iOS: NativeCleanShot.2024-04-27.at.05.08.03.mp4iOS: mWeb SafariSimulator.Screen.Recording.-.iPhone.15.Pro.-.2024-04-23.at.18.56.12.mp4MacOS: Chrome / SafariCleanShot.2024-04-23.at.18.41.15.mp4MacOS: DesktopCleanShot.2024-04-27.at.06.18.56.mp4 |
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.
@Skalakid It seems like the composer is missing a padding. th composer looks smaller compared to production.
Prod | PR |
---|---|
![]() |
![]() |
![]() |
![]() |
src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.tsx
Show resolved
Hide resolved
src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.tsx
Show resolved
Hide resolved
@fedirjh The problem with composer padding was fixed. Also, I removed some height calculations that were based on number of lines, since every line can have different |
Bug: IOS, composer jumps when reaching the third line, CleanShot.2024-04-23.at.19.10.45.mp4There is a delay when the user reaches the third line : CleanShot.2024-04-23.at.19.47.55.mp4 |
@shawnborton I can't reproduce it, emojis seem to have the correct size. However, the issue with cutting off emojis at the top is known and reported here I can only see that emojis aren't centered inside the composer, but it is also on the main and we are working on it |
These bugs are also present on the main branch: Screen.Recording.2024-04-25.at.12.54.26.mov |
Ah looks like we have some conflicts to resolve now |
I resolved the conflicts |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 1.4.67-7 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to staging by https://github.com/thienlnam in version: 1.4.68-0 🚀
|
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 1.4.67-7 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/francoisl in version: 1.4.68-3 🚀
|
Details
Fixed Issues
$ #39267
PROPOSAL:
Tests
2 .Go to any chat
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov