-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Add inline images preview to the Live Markdown Input on the web #50047
Add inline images preview to the Live Markdown Input on the web #50047
Conversation
Taking a look 👀 |
@Skalakid I still can reproduce the mentioned box issue on mWeb Screen.Recording.2024-10-03.at.16.05.19.mov |
9c9c06a
to
4f04d3d
Compare
I still can reproduce this issue: Screen.Recording.2024-10-03.at.17.25.15.movAlso if we send an image with a text, we can edit it and a broken image preview will be shown. Do you think we should fix it here as well? Screen.Recording.2024-10-03.at.17.26.56.mov |
@hungvu193, can you please check if you pulled the latest changes on my branch and delete |
@hungvu193 The second issue you reported is more complex since the image preview isn't displayed because the auth token is missing. It's pretty problematic, and we should think about how to solve this problem safely without exposing any token |
I think that's caches issue, I removed the |
All good! @thienlnam Can you add build label and ask QA to verify this PR? |
@hungvu193 I just fixed the issue with image previews that require auth token |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
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.
Confirmed forward delete works as expected in this branch. Somehow it's even worse in main - it now jumps the cursor to the end of the line 🙈
So if we can't get this merged soon or it gets reverted again, please let's do a small PR to just fix forward delete so we don't delay that further 🙏
please remove my review request, i have also verified #48797 is fixed with this PR |
Okay thanks, will request one more QA review so we can try to get ahead of any blockers |
Timing works out great since I'm also the deployer this week 😄 |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel! Task –Task description content with exceeded character limit flashes if tap on fix the errorsVersion Number: 9.0.44-8 Action Performed:Go to https://50047.pr-testing.expensify.com/home Expected Result:Task description content not flashes Actual Result:Task description content flashes Workaround:Unknown Platforms:Which of our officially supported platforms is this issue occurring on?
Screenshots/VideosBug6627475_1728329676357.TD.mp4Upwork Automation - Do Not Edit |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel! Task - & is displayed as & in task conversation createdVersion Number: 9.0.44-8 Action Performed:
Expected Result:& must not be displayed as & in task conversation created Actual Result:& is displayed as & in task conversation created Workaround:Unknown Platforms:Which of our officially supported platforms is this issue occurring on?
Screenshots/VideosBug6627535_1728332337594.Screenrecorder-2024-10-08-01-42-57-963_compress_1.mp4Upwork Automation - Do Not Edit |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel! Chat - Infinite loading when trying to reproduce video after sending it to chatVersion Number: 9.0.44-8 Action Performed:
Expected Result:After sending a video to chat, the user should be able to reproduce it by clicking on play on the preview. Actual Result:After uploading a video to chat, waiting a few seconds and clicking on play to reproduce it, the video loads infinitely. Workaround:Unknown Platforms:
Screenshots/VideosBug6627906_1728355612752.Video_Infinite.mp4Upwork Automation - Do Not Edit |
@hungvu193 @thienlnam Task - & is displayed as & in task conversation created and Chat - Infinite loading when trying to reproduce video after sending it to chat issues can also be reproduced on the latest main. The Task –Task description content with exceeded character limit flashes if tap on fix the errors issue is connected to #45885 (comment) since the input is scrolling element into view after clicking the "Fix" button. It's a pretty small bug, and we can handle it in the separate Live Markdown library bump |
Agree with you 💯 |
Yup I agree, I think QA is still running through regression testing though so let's see if anything else comes up that is worth fixing |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel! Image preview reloads when deleting line and returning to the previous line with image previewVersion Number: 9.0.44-8 Action Performed:
Expected Result:The image preview will not reload when deleting the line and returning to the previous line with image preview Actual Result:The image preview reloads when deleting the line and returning to the previous line with image preview Workaround:Unknown Platforms:
Screenshots/VideosBug6629630_1728483916910.20241009_221853.mp4Upwork Automation - Do Not Edit |
Thank you for reporting the Image preview reloads when deleting the line and returning to the previous line with the image preview issue. Here, I created the fix for it. However, I think it's a pretty low-priority bug, and we can add this fix to the E/App in a separate PR with a library bump |
Given that nothing major has come up, going to ship this today - and hopefully if anything comes up on the weekend that proves to be a blocker we can address it |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.49-0 🚀
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.0.49-2 🚀
|
Details
This PR bumps the version of the
react-native-live-markdown
to the latest and enables the inline image previews inside the input on the webFixed Issues
$ #40181 (comment)
$ #48797
PROPOSAL:
Tests
Inline image preview
#48797
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
web only feature
Android: mWeb Chrome
chrome.mov
iOS: Native
web only feature
iOS: mWeb Safari
safari.mov
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov