feat: HTML to Markdown parser integration #3229
Conversation
Screen.Recording.2021-06-23.at.12.49.32.AM.mov |
|
@roryabraham this PR is ready for review |
| @@ -153,7 +154,8 @@ class ReportActionContextMenu extends React.Component { | |||
| */ | |||
| getActionText() { | |||
There was a problem hiding this comment.
Let's change the method description here to:
Gets the markdown version of the message in an action.
There was a problem hiding this comment.
Bump – this method doc is no longer accurate
There was a problem hiding this comment.
Actually, I think we should make this change in ReportActionItemMessageEdit instead of ReportActionContextMenu, because this doesn't work in the case where you edit the previous message using the Up arrow key:
- Send a multiline message
- Press the
Uparrow key - Observe that the draft message in the
ReportActionItemMessageEditdoes not appear in multiple lines.
|
Hi @roryabraham I have updated the PR. Is this the correct approach, let me know for any changes. Peek.2021-06-23.22-44.mp4 |
There was a problem hiding this comment.
Not exactly correct – right now we're saving the draft message using the plain-text version of the message (i.e: reportAction.message.text). See the getAction function in ReportActionContextMenu here. Instead, I think we'll want to start passing the reportAction.message.html to saveReportActionDraft.
Once you've done that, the draftMessage prop of ReportActionItemMessageEdit will be html instead of text, and we shouldn't need the reportAction prop there. Then the only other change should be that we perform the HTML -> MD transform on the draftMessage prop of ReportActionItemMessageEdit in the constructor.
|
I have updated the PR |
|
@roryabraham I have updated all the required screen recordings & updated the PR as well |
| @@ -153,7 +154,8 @@ class ReportActionContextMenu extends React.Component { | |||
| */ | |||
| getActionText() { | |||
There was a problem hiding this comment.
Bump – this method doc is no longer accurate
|
@roryabraham can we merge this soon to avoid any merge 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. |
|
Unfortunately the deploy comments are not working right now. This was deployed to staging yesterday. |
TODO merge this Expensify/expensify-common#381 and then edit commit hash
Details
A small step towards migration to markdown and deprecation of
textFixed Issues
Fixes #2847
Tests
QA Steps
The structure of the message should remain the same i.e. with new lines
Tested On
Screenshots
Web
Peek.2021-06-25.21-14.mp4
Mobile Web
Peek.2021-06-25.21-16.mp4
Desktop
Screen.Recording.2021-06-25.at.10.29.34.PM.mov
iOS
Screen.Recording.2021-06-25.at.10.24.34.PM.mov
Android
Peek.2021-06-25.21-34.mp4