-
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
Enable Live Markdown Preview on web #38152
Enable Live Markdown Preview on web #38152
Conversation
@parasharrajat Please 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] |
src/components/Composer/index.tsx
Outdated
@@ -1,14 +1,15 @@ | |||
import {MarkdownTextInput} from '@expensify/react-native-live-markdown'; |
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.
Let's use RNMarkdownTextInput
component instead. https://github.com/Expensify/App/blob/main/src/components/RNMarkdownTextInput.tsx
I will get this reviewed in a couple of hours. Currently AFK |
Adding the original reviewers of the live markdown web PR that we reverted |
This comment has been minimized.
This comment has been minimized.
Should the email be clickable in the composer? Both email and web links are not clickable in composer. emailNotClickable.mp4 |
Maximum 3 level indents in composer for quote against 4 levels in the sent message. multiIndents.mp4 |
Can you explain what you mean? |
The scroll does not extend throughout the composer height when clicked on the expand icon on the top left of the composer. It works fine on scrollNotExtendedThroughoutComposerHeight.mp4 |
Should be fine now |
Okay generating one last adhoc for those new changes - let's merge this if we don't find any blockers. I've been adding these other edge cases to these issues that aren't blockers #39006 |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
Screen.Recording.2024-03-27.at.10.20.52.PM.movBUG: Undo operation removes more than one character |
Screen.Recording.2024-03-27.at.10.24.49.PM.movBUG: Unable to undo pasted quoted text. |
@shubham1206agra I think it's not a bug. History items are added with the debouce, so if you are writing faster, parts of the text are saved and then undone |
Also fixed 🚀 |
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.
This has gone through a series of reviews and we've fixed any main blockers - I tested one final time and everything looks good - let's ship this out and fix the follow ups as they come up
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Important QA team - please test Android & iOS as well since this PR also includes the upgrade of the package itself |
🚀 Deployed to staging by https://github.com/thienlnam in version: 1.4.58-0 🚀
|
🚀 Deployed to production by https://github.com/Beamanator in version: 1.4.58-8 🚀
|
useEffect(() => { | ||
if (!textInput.current || prevScroll === undefined) { | ||
return; | ||
} | ||
textInput.current.scrollTop = prevScroll; | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, [isComposerFullSize]); |
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.
Coming from #49541
When isComposerFullSize
is toggled, the scroll position aligns with the top of the compose bar. However, it would be more intuitive if the scroll position followed the bottom of the compose bar instead. This adjustment would ensure that the last line of text remains visible, preventing the user from losing their place when toggling the text box.
Proposal: #49541 (comment)
PR: #50520
Important
QA team - please test Android & iOS as well since this PR also includes the upgrade of the package itself
Details
This PR adds Live Markdown Preview feature for web using @expensify/react-native-live-markdown library.
Fixed Issues
$ #27977
PROPOSAL:
Tests
Testing Replacement Rules
Code Fence:
Type:
your text
Expected: Text appears in a separate block formatted as code.
Inline Code Block:
Type:
your text
Expected: Text appears inline, formatted as code.
Email Link:
Type: [email protected]
Expected: Email address turns into not clickable mailto link.
URL Link:
Type:
https://github.com/Expensify/App/pull/34292
Expected: Displays
https://github.com/Expensify/App/pull/34292
styled as web link (it shouldn't be clickable).Italics
Type: your text
Expected: Text appears in italics.
Bold Text:
Type: your text
Expected: Text appears in bold.
Strikethrough Text:
Type:
your textExpected: Text appears with a strikethrough.
Block Quote:
Type: > your text
Expected: Text appears as a block quote.
Heading:
Type: # Your Heading
Expected: Text appears as a large, bold heading.
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
Uploading live-markdown-for-web.mov…
MacOS: Desktop