Skip to content
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

[HOLD for payment 2024-10-24] [Live Markdown] Show the inline image in the live preview #40181

Open
thienlnam opened this issue Apr 12, 2024 · 69 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design NewFeature Something to build that is a new item.

Comments

@thienlnam
Copy link
Contributor

thienlnam commented Apr 12, 2024

https://expensify.slack.com/archives/C0671JDRKQW/p1712266105476139?thread_ts=1711736045.870539&cid=C0671JDRKQW

Problem:

While the live markdown preview now successfully avoids exceptions when rendering inline images, it currently does not display these images as expected. Users expect to see a real-time preview of all markdown elements, including images, to ensure their content is formatted correctly before publishing or saving. This lack of functionality in the previewer impacts user confidence in the document's final appearance, leading to a disjointed editing experience.

Solution:

Enhance the live markdown previewer to correctly parse and display inline images within the markdown content. This will involve updating the markdown parsing logic to handle image URLs and embed them appropriately in the preview pane. The update should support a variety of image formats and ensure that image links are not broken during the preview.

Issue OwnerCurrent Issue Owner: @anmurali
@thienlnam thienlnam self-assigned this Apr 12, 2024
@thienlnam thienlnam added Weekly KSv2 NewFeature Something to build that is a new item. labels Apr 12, 2024
Copy link

melvin-bot bot commented Apr 12, 2024

Triggered auto assignment to @anmurali (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

Copy link

melvin-bot bot commented Apr 12, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

@thienlnam
Copy link
Contributor Author

cc @tomekzaw

Copy link

melvin-bot bot commented Apr 12, 2024

Triggered auto assignment to @dubielzyk-expensify (Design), see these Stack Overflow questions for more details.

@thienlnam
Copy link
Contributor Author

We'll need some design input here on how we should show the images in the composer for live markdown

@tomekzaw
Copy link
Contributor

We'd like to implement this feature in Live Markdown.

@dubielzyk-expensify
Copy link
Contributor

Started a discussion here. Think there's a few things to agree on before we move forward with it.

@anmurali
Copy link

@tomekzaw looks like we made a decision on that slack thread on how to proceed. Any updates here on progress since then?

@melvin-bot melvin-bot bot removed the Overdue label Apr 30, 2024
@tomekzaw
Copy link
Contributor

I've asked @Skalakid from SWM to prepare a PoC on web, he's working on it.

@thienlnam
Copy link
Contributor Author

Any update on this one @Skalakid?

@melvin-bot melvin-bot bot removed the Overdue label May 13, 2024
@Skalakid
Copy link
Contributor

Skalakid commented May 14, 2024

Hi @thienlnam, I'm still building inline images PoC. I had to take a quick break from that because I needed to fix some other critical bugs. Now, most of them are fixed and I'm fully focused on the inline images feature. I was able to add an image preview from the URL into the Live Markdown Input. When testing I noticed that there are some problems with cursor positioning and with image preview reloading. These problems are blocking us from adding this feature, especially the one with cursor positioning. We will need to make some web parser logic changes to fix them. The parser changes will enable us to add new markdown features quicker and easier and will allow us to use more styling options without introducing critical layout bugs.

@quinthar
Copy link
Contributor

@Skalakid what are the next steps, and your ETA?

@Skalakid
Copy link
Contributor

The current steps are:

  1. Change Live Markdown web parser logic
  2. Enable using all CSS styles in Live Markdown and prevent adding too many new lines while doing it
  3. Implement inline images

More info about ETA in Slack thread

@melvin-bot melvin-bot bot added the Overdue label May 22, 2024
@anmurali
Copy link

Update from @Skalakid 3hours ago here

I continued refactoring in Live Markdown for the web. By changing the web parser logic, I could simplify some of the most complex parts of the code and fix most of the new parser edge cases. Also, I started changing the cursor position setting algorithm and planning new markdown styling logic

@melvin-bot melvin-bot bot removed the Overdue label May 22, 2024
@melvin-bot melvin-bot bot added the Overdue label May 31, 2024
@Skalakid
Copy link
Contributor

Skalakid commented Sep 5, 2024

04.09.2024

Hi, the PR is waiting for the internal review. Once we merge it I will prepare the PR to the E/App

@Skalakid
Copy link
Contributor

10.09.2024

Hi, the PR is still being reviewed and we are fixing all blocker bugs and possible regressions. We are trying our best to merge it as soon as possible

@Skalakid
Copy link
Contributor

Skalakid commented Sep 13, 2024

13.09.2024

Hi, the inline image previews has been finally merged into the library - PR! I will create the PR to the E/App with the bump 😄

@Skalakid
Copy link
Contributor

16.09.2024

Hello, here I created a PR to the E/App that enables inline images preview on web

@hungvu193
Copy link
Contributor

hungvu193 commented Sep 20, 2024

@thienlnam I review #49250, can you please assign me to this issue? TIA.

@Skalakid
Copy link
Contributor

Skalakid commented Oct 2, 2024

Hello, since the original PR was reverted, I will create another one that will fix reported critical issues

@Skalakid
Copy link
Contributor

Skalakid commented Oct 2, 2024

Here is a new PR to the E/App that enables inline image preview feature on web

@hungvu193
Copy link
Contributor

Here is a new PR to the E/App that enables inline image preview feature on web

I'll take a look at it today

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 4, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 17, 2024
@melvin-bot melvin-bot bot changed the title [Live Markdown] Show the inline image in the live preview [HOLD for payment 2024-10-24] [Live Markdown] Show the inline image in the live preview Oct 17, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Oct 17, 2024
Copy link

melvin-bot bot commented Oct 17, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Oct 17, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.49-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-10-24. 🎊

For reference, here are some details about the assignees on this issue:

  • @tomekzaw does not require payment (Contractor)

Copy link

melvin-bot bot commented Oct 17, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@tomekzaw] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@anmurali] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Oct 23, 2024
@melvin-bot melvin-bot bot removed the Overdue label Oct 23, 2024
@thienlnam
Copy link
Contributor Author

We'll also need a C+ paying to @hungvu193 here as well

Copy link

melvin-bot bot commented Oct 24, 2024

Payment Summary

Upwork Job

  • Reviewer: @hungvu193 owed $250 via NewDot
  • Contributor: @tomekzaw is from an agency-contributor and not due payment

BugZero Checklist (@anmurali)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design NewFeature Something to build that is a new item.
Projects
Status: HIGH
Development

No branches or pull requests

9 participants