Skip to content

[Tracking Issue] Implement HTML -> Markdown conversions in Expensimark #3047

Closed

Description

@roryabraham

Problem

Expensify.cash uses the Expensimark library in the expensify-common repo to convert markdown to HTML. That way, a user can compose a message using markdown, and it can be displayed + stored using HTML. However, when a user later goes to edit the message, we currently have no way to take the HTML and display it to them as markdown. We also don't want to store the original markdown in our database.

Why this is important

Editing comments is an important usability feature in Expensify.cash. However, we need to make sure that the edit-comment experience is as similar as possible to the original message composition experience.

Solution

Implement bi-directional HTML <---> Markdown conversion in Expensimark for all of the elements we currently support for Markdown ---> HTML conversion:

Activity

  1. changed the title [-][Tracking Issue] Implement HTML -> Markdown conversions in Expensimark[/-] [+][HOLD][Tracking Issue] Implement HTML -> Markdown conversions in Expensimark[/+] on May 21, 2021
  2. roryabraham commented on May 21, 2021

    @roryabraham
    ContributorAuthor

    HOLDing this on a POC PR over here. We might need to do an internal PR to update our other repos that also use Expensimark, depending on how we change the Expensimark interface.

  3. jasperhuangg commented on Jun 7, 2021

    @jasperhuangg
    Contributor

    @roryabraham I was just thinking about this–and sorry if this has already been considered–but why don't we just store both the HTML and markdown versions of the report comment, and switch between the two for editing/copying report comments?

    To minimize space, we can only store the HTML if the markdown and the HTML are identical (and default to it if the markdown version doesn't exist).

    Feel like this would be a whole lot simpler than doing a bunch of string manipulation on HTML to make it look like markdown–we'd probably also avoid a bunch of bugs this way.

  4. parasharrajat commented on Jun 7, 2021

    @parasharrajat
    Member

    @jasperhuangg has raised a very good point. But I think it in another way. I would say that we should save the MD as I believe converting HTML to markdown will loose original MD markup which user entered.

    And we can always convert the MD to HTML uniformly.

  5. roryabraham commented on Jun 7, 2021

    @roryabraham
    ContributorAuthor

    I don't necessarily disagree @jasperhuangg, but this was discussed in slack here. If you'd like to reopen the discussion, I think Slack would be the best place for that so it gets better visibility. The reason we didn't go with that solution is that it will nearly double the amount of data we need to store in our largest table.

    But @parasharrajat has introduced an interesting solution I don't believe was discussed before – only store the plain-text (markdown) version of a comment, then convert it to HTML only when needed.

  6. roryabraham commented on Jun 18, 2021

    @roryabraham
    ContributorAuthor

    Raised the proposal to store only markdown in this slack thread.

  7. roryabraham commented on Jun 21, 2021

    @roryabraham
    ContributorAuthor

    Moving forward with the original plan, I'm taking this off hold.

  8. changed the title [-][HOLD][Tracking Issue] Implement HTML -> Markdown conversions in Expensimark[/-] [+][Tracking Issue] Implement HTML -> Markdown conversions in Expensimark[/+] on Jun 21, 2021
  9. roryabraham commented on Jun 21, 2021

    @roryabraham
    ContributorAuthor

    For reference, this PR was merged as a POC, and it's being integrated into E.cash here.

  10. self-assigned this
    on Jun 21, 2021
  11. parasharrajat commented on Jul 14, 2021

    @parasharrajat
    Member

    @Jag96 We can mark finished tags on the list.

  12. Jag96 commented on Jul 14, 2021

    @Jag96
    Contributor

    Marked off the finished tags, left <br> unchecked and linked the bug we found

  13. parasharrajat commented on Jul 20, 2021

    @parasharrajat
    Member

    @Jag96 Let's open another issue for other tags. I see many complaints about it.

  14. isagoico commented on Jul 20, 2021

    @isagoico

    Make hyperlinks editable in chat comments

    Action Performed

    1. Send this a link using the hyperlink markdown to a conversation
    2. Click on edit the comment and save without making any changes

    Expected result

    Hyperlink markdown should be visible when editing the comment

    Actual result

    Hyperlink markdown is removed when editing the comment and then it's saved as plain text.

    Notes/images/Video

    Recording.198.mp4

    From @joaniew https://expensify.slack.com/archives/C01GTK53T8Q/p1626810543044700

    When I edit a chat that has a link to it already, the link turns into plain text and removes the hyper link, and I can’t edit it again to make it a link

  15. 6 remaining items

  16. roryabraham commented on Aug 11, 2021

    @roryabraham
    ContributorAuthor

    Looks to me like everything except #4188 is done or almost done

  17. parasharrajat commented on Aug 11, 2021

    @parasharrajat
    Member

    That is also done.

  18. roryabraham commented on Aug 24, 2021

    @roryabraham
    ContributorAuthor

    Looks like all the issues on our checklist are done here. I think I am going to ask Applause to do some extra QA here before we close this out and call it done.

  19. roryabraham commented on Aug 24, 2021

    @roryabraham
    ContributorAuthor

    Additional QA steps I would like to have done:

    1. Create a message with each of the markdown features in the issue description above.
    2. Attempt to edit the message. Verify that the markdown you entered when creating the message reappears in the edit box.
    3. Perform additional exploratory QA around editing messages containing markdown in New Expensify. Try to break it and let us know how it goes. 🙃
  20. roryabraham commented on Aug 24, 2021

    @roryabraham
    ContributorAuthor

    QA requested in Slack

  21. roryabraham commented on Sep 1, 2021

    @roryabraham
    ContributorAuthor

    Bumped Applause in slack here

  22. roryabraham commented on Sep 1, 2021

    @roryabraham
    ContributorAuthor

    Cool, actually Applause completed the exploratory testing we requested and exported these follow-up issues:

    Since those are all being processed by our engineering pipeline independently, I think we can close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions