search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.35k stars 2.78k forks source link

[HOLD https://github.com/Expensify/react-native-live-markdown/pull/394][Live Markdown] Support inline code background style #39518

Open thienlnam opened 5 months ago

thienlnam commented 5 months ago

https://expensify.slack.com/archives/C049HHMV9SM/p1712094742944359

Now that live markdown preview is out on production - let's add support for the background style for inline code

It should use the same font and background style, but currently doesn't.

image

melvin-bot[bot] commented 5 months ago

Triggered auto assignment to @lschurr (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

melvin-bot[bot] commented 5 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 5 months ago

Added to the main tracking issue as part of HIGH: https://github.com/Expensify/App/issues/36071

lschurr commented 5 months ago

Anything needed from BZ on this one @thienlnam?

thienlnam commented 5 months ago

Not at this time, we'll probably need payment to the C+ that reviews in the future but that's further away

tomekzaw commented 5 months ago

We're working on it.

iOS

I've asked @maksg from SWM to implement the iOS part. Here's the draft PR to Live Markdown:

Android

@maksg from SWM is also working on the Android part.

Web

@Skalakid had a working PoC some time ago, I will ask him to submit a PR for web.

lschurr commented 5 months ago

Any update @thienlnam @tomekzaw?

Skalakid commented 5 months ago

Hi @lschurr, @BartoszGrajdek is working on it

BartoszGrajdek commented 5 months ago

Hi @lschurr so here's a quick update on what's happening:

There are 2 different blocks to handle here contrary to what the issue title/description may say namely: inline code & pre blocks (single / triple backticks).

Like we agreed here there will be some changes to ExpensiMark to limit the number of cases where the pre styling is applied - I have it roughly done, but we will likely need some additional refactors.

Currently, I'm working on implementing the pre block styles for web. That's a tricky case because of the limitations that the Live Markdown puts on us, but I'm trying to overcome it.

I'll keep you updated! πŸ˜„

tomekzaw commented 5 months ago

As for the Android part, @maksg is working on it.

quinthar commented 4 months ago

Hi, what's the next step and ETA on this?

tomekzaw commented 4 months ago

Here's the Android PR from @maksg:

Currently waiting for @BartoszGrajdek's PR for web

Next step: Complete PR for web

ETA: ~2 weeks

lschurr commented 4 months ago

Any update @BartoszGrajdek @tomekzaw?

BartoszGrajdek commented 4 months ago

I've just added a PR to the expensify-common with some changes to how we treat code blocks. I'm currently solving the remaining bugs on Live Markdown for web to get it working on all browsers - after that we can move forward with the process of getting implementations for all platforms merged πŸ˜„ @lschurr

dangrous commented 4 months ago

PR has been merged (accidentally, by me, due to the merge freeze whoops)! So soon this will be good to go but don't merge any package.json changes yet! Also adding myself here for any further assistance since I think the issue linking got weird.

lschurr commented 3 months ago

Any update @BartoszGrajdek @dangrous?

BartoszGrajdek commented 3 months ago

Hi, it took us more time than we anticipated, mainly due to some bugs we have found that turned out to block us here. We're hoping to close this one in the coming days. There is one PR that will resolve an issue in the ExpensiMark itself which came to our attention after some extensive testing (I'll tag @dangrous to review it once it's ready). πŸ™πŸ»

@Skalakid is currently testing it together with the web PR. Once we make sure there are no additional bugs we'll push the expensify-common PR & later all the PRs that are waiting in the react-native-live-markdown lib. πŸ€“

Starting today I'll give you daily updates to keep you posted, we're getting really close! 🀞🏻

cc @lschurr

BartoszGrajdek commented 3 months ago

Status for today - MichaΕ‚ has found 2 small bugs during the testing. I'm already onto them hoping to solve both tomorrow πŸ˜„

BartoszGrajdek commented 3 months ago

Status 5.06.2024

We've managed to track the root cause for both of the bugs. One is a regression created by another PR, the second one has appeared because of the changes we've made and it's almost fixed now. Since none of them are related to ExpensiMark itself I opened up the PR for review (@dangrous if you have the capacity please go ahead & review it πŸ™πŸ» ).

BartoszGrajdek commented 3 months ago

Status 6.06.2024

I was debugging quite a few things today. We currently have ~2 other blockers at hand that need to be solved before we can add any additional changes, but I managed to work on regression fixes for a bit.

BartoszGrajdek commented 3 months ago

Status 10.06.2024

Didn't forget about this issue, right now we're solving some of the bugs that came up during the bump of react-native-live-markdown so I'm busy with that πŸ‘€

BartoszGrajdek commented 3 months ago

Status 11.06.2024-13.06.2024

Giving an update here, since we have discussed this issue internally at length after encountering all of the regressions/bugs that we've been fighting for the past week. We think that the best course of action will be to wait temporarily for a refactor that @Skalakid is working on. This will allow us to implement a much cleaner solution on the web that won't be susceptible to regressions.

I'll try to regularly come back here and let you know what's going on πŸ™ŒπŸ»

thienlnam commented 3 months ago

@BartoszGrajdek Is there a PR we can link to that we're waiting on?

BartoszGrajdek commented 3 months ago

Of course, here you go: https://github.com/Expensify/react-native-live-markdown/pull/394 πŸ™ŒπŸ»

lschurr commented 2 months ago

On hold

lschurr commented 2 months ago

Hold

thienlnam commented 2 months ago

Held PR still in the works

BartoszGrajdek commented 1 month ago

Hi! Adding an update here, since a few things have happened. πŸ‘€

As you know we already have the changes for Android/iOS prepared. At the beginning of this week, I finished a web implementation that I had been working on for the past ~2 weeks and it works really well in our refactored web component structure. That means everything's ready waiting for the refactor in question to get merged.

We're launching (hopefully) the last round of testing for the live markdown refactor PR and our current focus is to get it done ASAP. After that, if no regressions arise we'll work on merging all PRs inline-code / codefence related.

I'm OOO together with @Skalakid till 12.08, so I'll update you once we're back and know the test results. πŸ™Œ

lschurr commented 3 weeks ago

Anything new @BartoszGrajdek or are we still on hold?

dangrous commented 3 weeks ago

shush melvin

tomekzaw commented 1 week ago

We're working on it in react-native-live-markdown: