fix inline code blocks for native platforms #2307

parasharrajat · 2021-04-08T21:24:00Z

Details

Problem

We (Including other contributors) tried using a combination of Text & View tag to achieve the above UI but there are a couple of issues as follows:

Text is rendered as a block box but we want inline text.
In Another way, we can get the inline text but border and border-radius are not being rendered.

How I fixed it?

I had an idea of tokenizing the text. Then apply the style selectively. This is working well. I have made a small snap here.

we create one wrapper component which divides the text into tokens(words) divided based on word break. This component put the text tokens inline into a view and renders it.
We can then pass styles selectively to tokens and manage the border-radius for the first and last tokens.

Fixed Issues

Fixes #986

Tests / QA Steps

Type a sentence with code block formatting (single ticks) in the middle, like this:
This is a test to see how code renders in the middle of a sentence on mobile.
Notice that it should render inline like normal text.

Issue(Not blocker)

~~Inline Rendered code block is not vertically center with respect to the text.~~ ✔️ Fixed

Tested On

iOS
Android
Web
Desktop

Screenshots

Web / Desktop

Android

IOS

I don't have an IOS device thus I mimicked the logic here and which can be seen working well.

parasharrajat · 2021-04-08T22:10:21Z

@shawnborton I hope this style matches the desired one.

Luke9389 · 2021-04-08T22:46:57Z

Hey @parasharrajat, I'm noticing that this gets a bit clunky when there are longer words in the text. Here's what I'm talkin' about:

Think there's any way for us to fill in the white space with the code text style?

shawnborton · 2021-04-08T22:55:07Z

So if all of our message text is at 15px text size but with 20px line height, that means a normal looking piece of text should be 20px tall. I wonder if we could make our inline code (and code blocks for that matter) use a text size of 13px and a line height of 18px? This way the text feels more balanced, and when code wraps two lines, we get a small gap (2px) between the lines:

parasharrajat · 2021-04-08T23:19:07Z

@Luke9389. I not sure how to tackle this behavior. In this solution, each word is wrapped into a separate view, and then these views are being flex wrapped in the next line. but if a single word would be very big it will wrap to the next line leaving a gap in the previous line (Same behavior for Web). So far I have no idea of how to intelligently break that word into child views.

parasharrajat · 2021-04-08T23:50:06Z

Updated @shawnborton. Let me know if that works.

…asharrajat/quotedText

shawnborton · 2021-04-09T14:33:03Z

@parasharrajat did you update the screenshots as well?

parasharrajat · 2021-04-09T14:34:31Z

Oh no. I will update them. But for IOS , could you please test it. I am not a lot sure about that.

parasharrajat · 2021-04-09T18:07:58Z

@shawnborton I have updated the screens. Just I don't have IOS so I have put the image from snap.

shawnborton · 2021-04-09T19:46:47Z

Hmm it looks like there is a larger gap between lines that have regular text + inline code, versus lines that are all inline code:

parasharrajat · 2021-04-09T20:35:05Z

@shawnborton I tried a lot of ways for this. On IOS, the gap is equal but on Android, the inline code is vertically top relative to the text so I have to push the inline code towards the bottom to Keep it centered which Increases the gap where inline code comes on the same line with the text.

After many trials, I found this solution to add border and border-radius to the inline code which has this drawback on Android. Previously, we(including other proposals) were only able to achieve the background-color for the inline code so far. I am open to suggestions if someone can look at the code and help me improve it.

It seems that android does not handle it well natively but IOS can.

shawnborton · 2021-04-12T15:15:01Z

Going to tag my pal @marcaaron for thoughts on this from a technical perspective, as I know we've gone through this one a few times now.

It seems like we're getting close to the solution though. Is there any way you can make it so that both the normal text and inline code have a height of 20? If we can get the 20px tall containers to line up correctly, we hopefully can control the styles inside of them.

parasharrajat · 2021-04-12T15:57:29Z

I see that we set the line height of the whole text message then we can reach to almost equal gap but that text is not controlled by the inline code block and I don't think we don't want to change the line height for all messages. But let me see what I can do here.

shawnborton · 2021-04-12T16:49:41Z

Hmm I don't think I am quite following your comment. But I am pretty sure the line height for a default message is 20 right?

So my thinking is that if a block of text has a height/line height of 20, and then an inline code block is wrapped with a height of 20, those two elements should vertically align. Inside of the inline code block wrapper, we can make the code appear to be 18 tall with a margin top of 1 and a margin bottom of 1. Let me know if that makes sense.

marcaaron · 2021-04-12T18:26:59Z

Coming in a bit late to this PR and the overall direction and I'm curious... did we ever consider applying a solution like this in react-native-render-html? IIRC we had a conversation going with a maintainer where they suggested a similar approach cc @AndrewGable

As for styling advice, let's keep trying to achieve what we want for a bit. I don't have time to look into this today, but if we are still stuck down the line I'll jump in and see what I can do.

parasharrajat · 2021-04-14T21:01:37Z

@shawnborton So This is how it is rendered initially

Now to make the text inline I have to move the boxes some px downwards which works fine for the box which has normal inline text such as line 2 but when we have a whole line of code block then the negative margin which we gave to pull it down is not necessary and thus as a side effect it unnecessarily pull the whole line downwards and reducing the gap between this and next line.

shawnborton · 2021-04-15T15:32:24Z

Hmm okay. Are all of your changes pushed to your branch? Maybe I can pull it down and test on my end and let you know what I find.

parasharrajat · 2021-04-15T15:44:54Z

@shawnborton That would be really great. yeah its pushed.

shawnborton · 2021-04-19T21:22:44Z

I tried playing around with this but I am stumped. I added some helper background colors to see where things were breaking down:

It seems like when the message is only inline code, we can make it behave correctly in terms of making everything 20px tall, but the inner code staying at 18px with the 1px gap around it within the 20px container. Notice the nice 2px gaps when this goes multi line:

When we compare a message that is text-only and is two lines versus a message that is inline-code-only and two lines, the size is pretty similar, though it looks like the inline code is shifted upwards of about 2px:

But the biggest thing that seems to be breaking is when we have a combination of text and inline code that flows to multiple lines:

Notice how there is a big gap between the two red blocks? Ideally that entire block should be 40px tall but it's actually 48px tall... so somehow there is a mystery 8px being added, and my eyes are telling me that it's an extra 4px being added to the bottom of each line.

Anyways, I hope that is helpful in getting us closer, but I'm not entirely sure how to fix that mystery gap problem.

parasharrajat · 2021-04-19T21:28:39Z

Thanks for the detailed breakdown @shawnborton. If we can make the inline code block vertically center without passing the margin (this does not seem to be supported by native platforms), then this mystery could be resolved.

parasharrajat · 2021-04-20T21:29:42Z

@shawnborton @Luke9389. Breakthrough here. I have managed the gap. 🥳 🥳

shawnborton · 2021-04-20T21:31:16Z

Whoa, amazing Rajat!!

shawnborton · 2021-04-20T21:32:55Z

I think this is the behavior we want on all platforms, not just native. Is it possible to implement this everywhere?

parasharrajat · 2021-04-20T21:34:49Z

@shawnborton Yup.
here is web/ Desktop

shawnborton · 2021-04-20T21:36:17Z

Awesome, design-wise this looks great to me.

Luke9389

Hey Rajat, this looks awesome, nice job.
I put some knit-picky whitespace reviews in, but otherwise this looks great!

src/styles/styles.js

src/components/InlineCodeBlock/wrappedText.js

…harrajat/quotedText

parasharrajat · 2021-04-20T23:26:14Z

@Luke9389 Updated. Thanks

Luke9389

Nailed it Rajat, way to go!

OSBotify · 2021-04-21T00:22:26Z

🚀 Deployed to staging 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

isagoico · 2021-04-21T20:48:08Z

Chat - Sent code blocks are not properly centered

Expected Result

Sent message and code blocks are vertically centered/alligned

Actual Result

Code block is not centered vertically

Action Performed

Open staging.expensify.cash and login
Open any conversation
Send any message within ` , to make them code blocks

Platform

iOS ✔️

Build 1.0.28-0

Notes/Images/Video

roryabraham · 2021-04-23T00:56:46Z

Since this is a deploy blocker, the fix for which is still in review, I'm going to revert this PR. @parasharrajat what this means for you is that you'll need to resubmit a new PR that's a combination of this PR (#2307) and the fix for the deploy blocker (#2527).

That way, we can hopefully move forward with a production deploy as soon as possible. Let me know if you have any questions.

parasharrajat · 2021-04-23T01:02:54Z

Sure

OSBotify · 2021-05-08T01:43:19Z

🚀 Deployed to production in version: 1.0.39-5🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

s77rt · 2023-11-11T21:19:11Z

src/styles/styles.js

            fontFamily: fontFamily.MONOSPACE,
+            lineHeight: 18,
+            fontSize: 13,


The inline code block font size was being inherited. But after explicitly setting it to 13px, it's always rendered at 13px even if the parent is a H1 where the font size is expected to be a little bigger. (Coming from #30203)

fix: inline code blocks for native platforms

70179c7

parasharrajat requested a review from a team as a code owner April 8, 2021 21:24

MelvinBot requested review from Luke9389 and removed request for a team April 8, 2021 21:24

fix vertical alignment of text

71f68e6

parasharrajat added 2 commits April 9, 2021 05:17

styles inline code

92cebd7

gap in rows

ed7a139

Merge branch 'master' of github.com:Expensify/Expensify.cash into par…

441ffd7

…asharrajat/quotedText

fix: styles

7d88395

sketchydroide assigned sketchydroide and parasharrajat and unassigned sketchydroide Apr 13, 2021

parasharrajat added 2 commits April 21, 2021 02:09

fix: block styles

f764793

fix: cross platform styling

f5290a1

Luke9389 suggested changes Apr 20, 2021

View reviewed changes

parasharrajat added 2 commits April 21, 2021 04:38

chg: linting

80f3219

Merge branch 'main' of github.com:Expensify/Expensify.cash into paras…

92ddc5a

…harrajat/quotedText

Luke9389 approved these changes Apr 20, 2021

View reviewed changes

Luke9389 merged commit eba5df8 into Expensify:main Apr 20, 2021

OSBotify mentioned this pull request Apr 20, 2021

Deploy Checklist: Expensify.cash 2021-04-13 #2380

Closed

66 tasks

isagoico added the DeployBlockerCash This issue or pull request should block deployment label Apr 21, 2021

parasharrajat mentioned this pull request Apr 22, 2021

fix inline code blocks #2527

Merged

5 tasks

roryabraham mentioned this pull request Apr 23, 2021

Revert "fix inline code blocks for native platforms" #2550

Merged

s77rt mentioned this pull request Jun 3, 2023

[HOLD for payment 2023-06-19] [$1000] IOS - Chat - Top of the emoji inside the inline block is cut off #19922

Closed

6 tasks

s77rt mentioned this pull request Oct 25, 2023

[HOLD for payment 2023-11-17] [$500] No text size change on bold on single backtick code block #30203

Closed

6 tasks

s77rt reviewed Nov 11, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix inline code blocks for native platforms #2307

fix inline code blocks for native platforms #2307

parasharrajat commented Apr 8, 2021 •

edited

Loading

parasharrajat commented Apr 8, 2021

Luke9389 commented Apr 8, 2021

shawnborton commented Apr 8, 2021

parasharrajat commented Apr 8, 2021 •

edited

Loading

parasharrajat commented Apr 8, 2021

shawnborton commented Apr 9, 2021

parasharrajat commented Apr 9, 2021 •

edited

Loading

parasharrajat commented Apr 9, 2021

shawnborton commented Apr 9, 2021

parasharrajat commented Apr 9, 2021 •

edited

Loading

shawnborton commented Apr 12, 2021

parasharrajat commented Apr 12, 2021

shawnborton commented Apr 12, 2021

marcaaron commented Apr 12, 2021

parasharrajat commented Apr 14, 2021

shawnborton commented Apr 15, 2021

parasharrajat commented Apr 15, 2021

shawnborton commented Apr 19, 2021

parasharrajat commented Apr 19, 2021

parasharrajat commented Apr 20, 2021

shawnborton commented Apr 20, 2021

shawnborton commented Apr 20, 2021

parasharrajat commented Apr 20, 2021

shawnborton commented Apr 20, 2021

Luke9389 left a comment

parasharrajat commented Apr 20, 2021

Luke9389 left a comment

OSBotify commented Apr 21, 2021

isagoico commented Apr 21, 2021 •

edited

Loading

roryabraham commented Apr 23, 2021

parasharrajat commented Apr 23, 2021

OSBotify commented May 8, 2021

s77rt Nov 11, 2023

fix inline code blocks for native platforms #2307

fix inline code blocks for native platforms #2307

Conversation

parasharrajat commented Apr 8, 2021 • edited Loading

Details

Problem

How I fixed it?

Fixed Issues

Tests / QA Steps

Issue(Not blocker)

Tested On

Screenshots

Web / Desktop

Android

IOS

parasharrajat commented Apr 8, 2021

Luke9389 commented Apr 8, 2021

shawnborton commented Apr 8, 2021

parasharrajat commented Apr 8, 2021 • edited Loading

parasharrajat commented Apr 8, 2021

shawnborton commented Apr 9, 2021

parasharrajat commented Apr 9, 2021 • edited Loading

parasharrajat commented Apr 9, 2021

shawnborton commented Apr 9, 2021

parasharrajat commented Apr 9, 2021 • edited Loading

shawnborton commented Apr 12, 2021

parasharrajat commented Apr 12, 2021

shawnborton commented Apr 12, 2021

marcaaron commented Apr 12, 2021

parasharrajat commented Apr 14, 2021

shawnborton commented Apr 15, 2021

parasharrajat commented Apr 15, 2021

shawnborton commented Apr 19, 2021

parasharrajat commented Apr 19, 2021

parasharrajat commented Apr 20, 2021

shawnborton commented Apr 20, 2021

shawnborton commented Apr 20, 2021

parasharrajat commented Apr 20, 2021

shawnborton commented Apr 20, 2021

Luke9389 left a comment

Choose a reason for hiding this comment

parasharrajat commented Apr 20, 2021

Luke9389 left a comment

Choose a reason for hiding this comment

OSBotify commented Apr 21, 2021

isagoico commented Apr 21, 2021 • edited Loading

Chat - Sent code blocks are not properly centered

Expected Result

Actual Result

Action Performed

Platform

roryabraham commented Apr 23, 2021

parasharrajat commented Apr 23, 2021

OSBotify commented May 8, 2021

s77rt Nov 11, 2023

Choose a reason for hiding this comment

parasharrajat commented Apr 8, 2021 •

edited

Loading

parasharrajat commented Apr 8, 2021 •

edited

Loading

parasharrajat commented Apr 9, 2021 •

edited

Loading

parasharrajat commented Apr 9, 2021 •

edited

Loading

isagoico commented Apr 21, 2021 •

edited

Loading