Add inline images preview to the Live Markdown Input on the web #50047

Skalakid · 2024-10-02T10:51:34Z

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 web

Fixed Issues

$ #40181 (comment)
$ #48797

PROPOSAL:

Tests

Inline image preview

Open any chat room / page with Live Markdown input
Enter inline image markdown. For example:

*test* ![demo image](https://picsum.photos/id/1067/200/300)_test_

Verify the behavior of the input with image preview inside
Verify the loading indicator styling and layout before the image loads
Verify the size and aspect ratio of the image preview
Verify other cases where image can break the input

#48797

Initiate a DM with any user
Compose a message
Place the cursor in the middle of the composed message
Press the Delete button on the keyboard

Verify that no errors appear in the JS console

Offline tests

Same as Tests

QA Steps

Same as Tests

Verify that no errors appear in the JS console

PR Author Checklist

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

hungvu193 · 2024-10-03T02:00:11Z

Taking a look 👀

hungvu193 · 2024-10-03T09:06:31Z

@Skalakid I still can reproduce the mentioned box issue on mWeb

Screen.Recording.2024-10-03.at.16.05.19.mov

hungvu193 · 2024-10-03T10:29:20Z

I still can reproduce this issue:
#49973

Screen.Recording.2024-10-03.at.17.25.15.mov

Also 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

Skalakid · 2024-10-03T10:37:34Z

@hungvu193, can you please check if you pulled the latest changes on my branch and delete node_modules before npm i, please? If it doesn't help please give me test steps how to reproduce the first issue

Skalakid · 2024-10-03T12:13:43Z

@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

hungvu193 · 2024-10-03T14:19:05Z

@hungvu193, can you please check if you pulled the latest changes on my branch and delete node_modules before npm i, please? If it doesn't help please give me test steps how to reproduce the first issue

I think that's caches issue, I removed the node_modules and run npm ci and this issue's gone 🤦

hungvu193 · 2024-10-03T14:35:59Z

All good!

@thienlnam Can you add build label and ask QA to verify this PR?

…iews with aut token support

Skalakid · 2024-10-04T08:39:40Z

@hungvu193 I just fixed the issue with image previews that require auth token

melvin-bot · 2024-10-04T08:39:47Z

@dubielzyk-expensify @ishpaul777 One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

hungvu193 · 2024-10-04T08:42:31Z

Please ignore this PR @ishpaul777

hungvu193 · 2024-10-04T09:05:42Z

@hungvu193 I just fixed the issue with image previews that require auth token

Nice! it works 🚀

hungvu193 · 2024-10-04T09:33:11Z

@Skalakid I think the recent changes from selection logic breaks shortcut key, now when I press command + A to select all, the cursor will move to the first character of the line:

Screen.Recording.2024-10-04.at.16.32.29.mov

hungvu193 · 2024-10-04T09:38:26Z

another way to reproduce it, type a letter and double click to select it, you can notice that you can select that letter.

Skalakid · 2024-10-04T12:27:01Z

@hungvu193 fixed!

hungvu193 · 2024-10-04T15:34:59Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

Screen.Recording.2024-10-04.at.22.32.30.mov

Screen.Recording.2024-09-20.at.22.34.58.mov

iOS: Native

iOS: mWeb Safari

Screen.Recording.2024-09-20.at.22.25.19.mov

Screen.Recording.2024-10-04.at.22.34.29.mov

MacOS: Chrome / Safari

Chrome.mov

MacOS: Desktop

Screen.Recording.2024-09-20.at.22.23.07.mov

hungvu193 · 2024-10-04T15:35:54Z

Please add build label and ask QA team to verify this PR. Ty 🙇

github-actions · 2024-10-04T17:03:05Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/50047/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/50047/index.html

Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/50047/NewExpensify.dmg	https://50047.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

rafecolton

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 🙏

ishpaul777 · 2024-10-04T20:32:00Z

please remove my review request, i have also verified #48797 is fixed with this PR

thienlnam · 2024-10-07T17:08:03Z

Okay thanks, will request one more QA review so we can try to get ahead of any blockers

thienlnam · 2024-10-07T17:09:56Z

Timing works out great since I'm also the deployer this week 😄

lanitochka17 · 2024-10-07T20:06:42Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Task –Task description content with exceeded character limit flashes if tap on fix the errors

Version Number: 9.0.44-8
Reproducible in staging?: N
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Go to https://50047.pr-testing.expensify.com/home
2. Log in
3. Open any chat
4. Tap on +/ Assign task
5. Enter task title
6. Enter description longer than 1000 characters and tap Next
7. When focus is gone tap on fix the errors link

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/Videos

Bug6627475_1728329676357.TD.mp4

Upwork Automation - Do Not Edit

lanitochka17 · 2024-10-07T21:16:07Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Task - & is displayed as & in task conversation created

Version Number: 9.0.44-8
Reproducible in staging?: N
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Go to https://50047.pr-testing.expensify.com/home
Open self DM
Enter [] @jaihanumanblog@gmail.com
&
Send the message

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/Videos

Bug6627535_1728332337594.Screenrecorder-2024-10-08-01-42-57-963_compress_1.mp4

Upwork Automation - Do Not Edit

Skalakid added 4 commits October 2, 2024 12:11

Bump react-native-live-markdown version to 0.1.159

51db8c9

Add styles for inline images

91dc1ed

Fix mention suggestion box position

a49a54d

Update Podfile

9aa424d

Skalakid mentioned this pull request Oct 2, 2024

[Live Markdown] Show the inline image in the live preview #40181

Open

ishpaul777 mentioned this pull request Oct 2, 2024

[$250] Forward delete moves cursor location after deleting a character #48797

Open

6 tasks

VickyStash mentioned this pull request Oct 3, 2024

Enlarge emojis in other contexts than just single character messages #47547

Open

50 tasks

Skalakid added 4 commits October 3, 2024 10:08

Merge branch 'main' into @Skalakid/inline-images-feature-v2

b257fd7

Fix mention suggestion box position when input is scrollable

ab3429a

Bump react-native-live-markdown version to 0.1.163

8c0c879

Update Podfile

7899bc8

Skalakid added 2 commits October 3, 2024 12:19

Fix getting cursor position y

a754fc4

Fix getting cursor position while typing text

4f04d3d

Skalakid force-pushed the @Skalakid/inline-images-feature-v2 branch from 9c9c06a to 4f04d3d Compare October 3, 2024 10:29

This was referenced Oct 3, 2024

Add authToken support for inline image URLs Expensify/react-native-live-markdown#510

Merged

[$250] Room - Pasting long text in description not scrolled to bottom & cursor shown at middle. #45885

Open

Skalakid added 2 commits October 4, 2024 07:19

Merge branch 'main' into @Skalakid/inline-images-feature-v2

547b2c6

Bump react-native-live-markdown version to 0.1.164 and add image prev…

37d6bae

…iews with aut token support

Skalakid marked this pull request as ready for review October 4, 2024 08:39

Skalakid requested review from a team as code owners October 4, 2024 08:39

melvin-bot bot requested a review from dubielzyk-expensify October 4, 2024 08:39

melvin-bot bot requested a review from ishpaul777 October 4, 2024 08:39

melvin-bot bot removed the request for review from a team October 4, 2024 08:39

VickyStash mentioned this pull request Oct 4, 2024

[$1000] Make emojis larger in other contexts than just single character messages #4114

Open

Fix selection shortcuts

8521ff5

hungvu193 approved these changes Oct 4, 2024

View reviewed changes

melvin-bot bot requested a review from rafecolton October 4, 2024 15:35

roryabraham added the Ready To Build label Oct 4, 2024

thienlnam self-requested a review October 4, 2024 17:25

rafecolton reviewed Oct 4, 2024

View reviewed changes

rafecolton removed the request for review from ishpaul777 October 4, 2024 20:33

Skalakid mentioned this pull request Oct 7, 2024

[$2000] Android/IOS - Split/request money - Chat composer is focused after IOU request from green plus button #10731

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Skalakid commented Oct 2, 2024 •

edited

Loading

hungvu193 commented Oct 3, 2024

hungvu193 commented Oct 3, 2024

hungvu193 commented Oct 3, 2024

Skalakid commented Oct 3, 2024 •

edited

Loading

Skalakid commented Oct 3, 2024

hungvu193 commented Oct 3, 2024

hungvu193 commented Oct 3, 2024

Skalakid commented Oct 4, 2024

melvin-bot bot commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

Skalakid commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

hungvu193 commented Oct 4, 2024 •

edited

Loading

github-actions bot commented Oct 4, 2024

rafecolton left a comment

ishpaul777 commented Oct 4, 2024

thienlnam commented Oct 7, 2024

thienlnam commented Oct 7, 2024

lanitochka17 commented Oct 7, 2024 •

edited

Loading

lanitochka17 commented Oct 7, 2024

Add inline images preview to the Live Markdown Input on the web #50047

Are you sure you want to change the base?

Add inline images preview to the Live Markdown Input on the web #50047

Conversation

Skalakid commented Oct 2, 2024 • edited Loading

Details

Fixed Issues

Tests

Inline image preview

#48797

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

hungvu193 commented Oct 3, 2024

hungvu193 commented Oct 3, 2024

hungvu193 commented Oct 3, 2024

Skalakid commented Oct 3, 2024 • edited Loading

Skalakid commented Oct 3, 2024

hungvu193 commented Oct 3, 2024

hungvu193 commented Oct 3, 2024

Skalakid commented Oct 4, 2024

melvin-bot bot commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

Skalakid commented Oct 4, 2024

hungvu193 commented Oct 4, 2024

Reviewer Checklist

Screenshots/Videos

hungvu193 commented Oct 4, 2024 • edited Loading

github-actions bot commented Oct 4, 2024

rafecolton left a comment

Choose a reason for hiding this comment

ishpaul777 commented Oct 4, 2024

thienlnam commented Oct 7, 2024

thienlnam commented Oct 7, 2024

lanitochka17 commented Oct 7, 2024 • edited Loading

Task –Task description content with exceeded character limit flashes if tap on fix the errors

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

lanitochka17 commented Oct 7, 2024

Task - & is displayed as & in task conversation created

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Skalakid commented Oct 2, 2024 •

edited

Loading

Skalakid commented Oct 3, 2024 •

edited

Loading

hungvu193 commented Oct 4, 2024 •

edited

Loading

lanitochka17 commented Oct 7, 2024 •

edited

Loading