Open thienlnam opened 2 months ago
melvin-bot[bot]
commented
2 months ago 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.
melvin-bot[bot]
commented
2 months ago :warning: 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 :warning:
thienlnam
commented
2 months ago cc @tomekzaw
melvin-bot[bot]
commented
2 months ago Triggered auto assignment to @dubielzyk-expensify (Design), see these Stack Overflow questions for more details.
thienlnam
commented
2 months ago We'll need some design input here on how we should show the images in the composer for live markdown
tomekzaw
commented
2 months ago We'd like to implement this feature in Live Markdown.
dubielzyk-expensify
commented
2 months ago Started a discussion here. Think there's a few things to agree on before we move forward with it.
anmurali
commented
2 months ago @tomekzaw looks like we made a decision on that slack thread on how to proceed. Any updates here on progress since then?
tomekzaw
commented
2 months ago I've asked @Skalakid from SWM to prepare a PoC on web, he's working on it.
thienlnam
commented
1 month ago Any update on this one @Skalakid?
Skalakid
commented
1 month ago 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
commented
1 month ago @Skalakid what are the next steps, and your ETA?
Skalakid
commented
1 month ago The current steps are:
More info about ETA in Slack thread
anmurali
commented
1 month ago 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
dubielzyk-expensify
commented
1 month ago I believe this is progressing
Skalakid
commented
1 month ago Hi, currently this issue is a bit blocked by Live Markdown web parser refactor. We need to finish it first so we will be able to add inline images without creating any critical regressions. When creating POC I discovered that current praser logic is limiting us and we can't use some of the most important styles. (like display: block) without creating cursor position or parsing bugs. Because of that we decided to refactor Live Markdown on web to simplify our codebase and improve it with the knowledge that we gained during the developement. New logic will allow us to add more complex features quicker and with less code. More info and my updates in this Slack thread
Skalakid
commented
4 weeks ago Hi, today I was refactoring cursor position-related functions. After changing web parser logic and html element structure inside the Live Markdown Input, we need to change how we get and set cursor index. Currently I'm testing out different approaches and creating the most optimal one
Skalakid
commented
4 weeks ago Hi, today I continued working on fixing cursor position-related functions in Live Markdown Input on the web. I implemented a new basic cursor position getting logic and discussed some refactor-related topics with the team
Skalakid
commented
3 weeks ago On the 7th of June I was OOO, and today I prioritized fixing new bugs that appeared in react-native-live-markdown. I will try finishing the cursor utils refactor tomorrow so we can unblock this issue
anmurali
commented
3 weeks ago Being worked on.
Skalakid
commented
3 weeks ago Hi, today I finished refactoring cursor utils inside the Live Markdown Input for the web. That means that all the most critical parts of code, that inline image PR depends, on are finished. Now, I will focus on adding the last changes to my refactor branch to clear it and make the main component more readable for external contributors. The end of refactoring is near and soon I will be focused on adding inline images feature thanks to new, refactored logic and structure
Skalakid
commented
3 weeks ago Hi, today I continued the Live Markdown refactor. Together with @BartoszGrajdek, we discussed what the next refactor steps should be. We decided to look through the main web markdown component and try to remove some old unnecessary logic and rewrite some functions to clarify them and prevent bugs in the future. Next to that, I worked today on changing the Live Markdown tree structure and fixing cursor position bugs that I found during testing
Skalakid
commented
3 weeks ago Hi, today I continued working on Live Markdown for the web. I had to make some changes in tree structure building logic, to always have an updated tree of elements. I will allow us to set the cursor position in all cases and make our implementation more flexible for future improvements. After that, I started writing some unit tests for current changes to make sure that everything works
Skalakid
commented
2 weeks ago Hi, yesterday I had to prioritize some other Live Markdown related tasks. But, today I continued the Live Markdown refactor and I fixed problems with the cursor position setting and started working on fixing bugs with new line insertion
Skalakid
commented
2 weeks ago Hi, today I was focused on fixing a huge blocker that appeared after changing the Live Markdown structure during the refactor. There was a problem with the parser while adding and removing newlines and also, the cursor position was being set at the wrong index. I was able to fix the main problem by enhancing cursor position algorithms and refactoring some functions in the main input component. However, there are still a couple of edge cases that we have to take care of before inline image feature implementation
dubielzyk-expensify
commented
2 weeks ago Not overdue. Looks like people are still working on it :)
Skalakid
commented
2 weeks ago Hi, today I finished fixing all web parser edge case bugs that appeared after the refactor. Here is a draft PR. Now, I will focus on:
and other things that will simplify our codebase. I think the PR should be ready to review next week and than we can start over inline image POC implementation
Skalakid
commented
1 week ago Hi, I was OOO on 21.06 and 24.06 so there was no update from me. Today I jumped back to Live Markdown refactor. Unfortunately, I encountered some commit signing problems after moving to the new Mac and I had to squash previously pushed commits into signed one. After fixing some problems on my side, now I'm focused on fixing markdown html DOM refreshing to improve performance. I want to add the same feature that we hade before refactor to refresh DOM only when user adds or delates markdown
Skalakid
commented
1 week ago Hi, today I was finishing web parser optimization to make input refresh its' structure only when the html dom structure changes. After that, I started cleaning up the code after the logic and structure changes - draft PR
Skalakid
commented
1 week ago Hi, I worked shorter today and focused on reviewing and testing Live Markdown-related PR. Also, I was updating tests in the web parser refactor PR
Skalakid
commented
1 week ago Hello, today I focused on finishing the Live Markdown refactor PR. I cleaned up the code, fixed the last bugs that appeared after the last changes, and gave the PR to the internal review :smile:
Skalakid
commented
4 days ago Hi, today I was testing the Live Markdown refactor in E/App. I found a couple of bugs and crashes, however I was able to fix them. After that, I started fixing a problem with copying and pasting text from Markdown Input. I will continue the investigation tomorrow
Skalakid
commented
2 days ago Hi, today I fixed all bugs connected to text pasting in my Live Markdown refactor. I changed the logic of how we handle paste events and how we position the cursor during text replacements on paste. Also, I've added a security fix to block HTML injections to our input component.
The Live Markdown refactor is near the finish. Now we are reviewing the code and testing our solution in E/App on different browsers. The code was fully refactored, cleared, and enhanced so we can add more complex features (like inline images) and improve code quality in our library. A more detailed description of all our changes and goals can be found in my PR description or this thread.
Here is a quick preview of the logic and structure changes that we were able to make:
https://github.com/Expensify/App/assets/39538890/ee570832-ebf6-48e0-ad07-62f276d1e308
Skalakid
commented
2 days ago Hi, today I started fixing e2e tests in web Live Markdown refactor PR that break after logic and structure changes. Thanks to that I discovered that the new implementation didn't work on the Firefox browser. There were many problems with cursor positioning and new line insertion caused by Firefox's different behavior. I managed to fix most of them. Tomorrow I will continue updating e2e tests and check if everything works
dubielzyk-expensify
commented
1 day ago Not overdue Melvin.
Skalakid
commented
1 day ago Hi, today I was fixing the e2e tests in the Live Markdown web refactor PR. Tomorrow I will make the last check to see if everything works fine after adding it to E/App and I will open the PR for C+ review
Skalakid
commented
5 hours ago Hi, today I focused on fixing the e2e tests that were failing on CI/CD and fixing TS errors in my implementation. Also, I discovered a small problem with multiline tag generation in the parser so I've added a fix for it. After that, I opened my PR for review. Unfortunately, there is one more thing that is blocking us from opening the PR to E/App - a text-pasting-related issue that occurs on the latest LM main. I started investigating it
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.