fix inline code blocks #2527

parasharrajat · 2021-04-22T02:20:21Z

Please review.

Details

This PR also includes the reverted changes as per the comment #2307 (comment)

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

the issue found here https://github.com/Expensify/Expensify.cash/pull/2307#issuecomment-825308917#2307#issuecomment-824347903

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.

Tested On

Screenshots

Web / Desktop

Mobile Web

Android

IOS

iPhone 11	iPhone 8

jasperhuangg · 2021-04-22T05:00:34Z

src/styles/codeStyles/index.android.js

+    borderTopLeftRadius: 0,
+    borderBottomLeftRadius: 0,
+    borderTopRightRadius: 0,
+    borderBottomRightRadius: 0,


Isn't this just borderRadius: 0? Why does this have to be included?

@jasperhuangg these style props take precedence over borderRadius. Due to the reason we need to specify borderRadius for Web, we can't remove it from the main code styles. But on native, the new implementation only wants border radius for the first and last word. Thus to override it, I have to be specific for each edge.
In Short, borderRadius does not work.

roryabraham

@parasharrajat Can we please DRY this up with the following module structure:

src/styles/codeStyles/
---| index.js    // web/desktop/mWeb
---| defaultNativeCodeStyles.js   // all the similar styles between iOS and Android
---| index.ios.js   // import defaultNativeCodeStyles and override what you need
---| index.android.js   // import defaultNativeCodeStyles and override what you need

Also, can you please test on all platforms and provide screenshots for each? Without more context, it seems weird to me that we would be exporting empty objects on web/desktop.

parasharrajat · 2021-04-23T01:01:57Z

Sure. These styles are only for native platforms as web support the styling natively.

I will provide screenshot for others as well.

parasharrajat · 2021-04-23T15:43:58Z

@roryabraham Merged the reverted changes here.

Screenshots updated. Thanks.

src/components/InlineCodeBlock/WrappedText.js

roryabraham · 2021-04-23T17:10:15Z

src/components/InlineCodeBlock/WrappedText.js

+
+                        // Outer View is important to vertically center the Text
+                        <View
+                                // eslint-disable-next-line react/no-array-index-key


Could you explain why this suppression is necessary? I'm confused because you are providing a key?

Oh. Sorry. Just made few adjustments and forget to Change it.

Actually, Eslint complains about an array index being used as key I know that is not the case but there is an error. So I had to

@parasharrajat Can you clarify what you mean here? What is the error?

The exact error was Array index can't be used as a key. so I have to disable it to use the array index in the key.

src/components/InlineCodeBlock/index.native.js

src/components/InlineCodeBlock/WrappedText.js

Co-authored-by: Rory Abraham <[email protected]>

…xpensify.cash into parasharrajat/quotedText

parasharrajat · 2021-04-23T18:28:22Z

Updated. Thanks.

parasharrajat · 2021-04-23T18:58:32Z

Updated.

jasperhuangg

Looks good, besides the issue with eslint-disable-next-line that Rory commented on.

parasharrajat · 2021-04-26T05:01:57Z

@jasperhuangg actually eslint complaints about using the array index into the key, even though I am using the combination to create unique key. So I had to disable the warnings.

parasharrajat · 2021-04-26T18:10:51Z

@roryabraham Waiting for your input here. Sorry tagged you in a different issue, previously.

roryabraham

Code LGTM and tested well 👍

parasharrajat · 2021-04-27T18:52:46Z

@jasperhuangg All yours.

parasharrajat · 2021-04-29T01:58:33Z

@jasperhuangg any input here.

jasperhuangg · 2021-04-30T02:06:47Z

LGTM! merging

OSBotify · 2021-04-30T02:35:21Z

🚀 Deployed to staging in version: 1.0.34-2🚀

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

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-06-13T19:32:35Z

src/styles/codeStyles/index.ios.js

+const codeTextStyle = {
+    lineHeight: 15,
+};


The line height here was not enough and caused a regression #19922 when the inline code contains emojis.

s77rt · 2023-06-13T19:34:18Z

src/components/InlineCodeBlock/WrappedText.js

+function getTextMatrix(text) {
+    return text.split('\n').map(row => _.without(row.split(/(\s)/), ''));
+}


This caused a regression #19922 when using emojis and text the inline code will look misaligned #19922 (comment).

fix: IOS Inline code block

fef2e14

parasharrajat requested a review from a team as a code owner April 22, 2021 02:20

MelvinBot requested review from jasperhuangg and removed request for a team April 22, 2021 02:20

jasperhuangg requested changes Apr 22, 2021

View reviewed changes

roryabraham requested changes Apr 23, 2021

View reviewed changes

roryabraham mentioned this pull request Apr 23, 2021

fix inline code blocks for native platforms #2307

Merged

4 tasks

parasharrajat added 3 commits April 23, 2021 12:06

fix: inline code block in IOS and refactor code

b5dabce

pull reverted changes

00c421b

new: inline coe block implementation for native platforms

b8181e0

parasharrajat changed the title ~~fix: IOS Inline code block's text is cropping~~ fix inline code blocks Apr 23, 2021

parasharrajat requested review from jasperhuangg and roryabraham April 23, 2021 15:44

roryabraham requested changes Apr 23, 2021

View reviewed changes

parasharrajat and others added 3 commits April 23, 2021 23:10

Updated Comment

a4b0011

Co-authored-by: Rory Abraham <[email protected]>

cleanup

d103104

Merge branch 'parasharrajat/quotedText' of github.com:parasharrajat/E…

2118943

…xpensify.cash into parasharrajat/quotedText

fix props for inline code block

b94e2ba

parasharrajat requested a review from roryabraham April 23, 2021 20:12

jasperhuangg reviewed Apr 26, 2021

View reviewed changes

roryabraham approved these changes Apr 26, 2021

View reviewed changes

parasharrajat requested a review from jasperhuangg April 26, 2021 18:40

parasharrajat mentioned this pull request Apr 28, 2021

Blocks of quoted text render incorrectly on mobile #986

Closed

jasperhuangg approved these changes Apr 30, 2021

View reviewed changes

jasperhuangg merged commit a656a1e into Expensify:main Apr 30, 2021

OSBotify mentioned this pull request Apr 30, 2021

Deploy Checklist: Expensify.cash 2021-04-29 #2639

Closed

23 tasks

jsamr mentioned this pull request Jul 16, 2021

[Performance] [Audit]: messages with Views nested in Text for inline code are 700% slower to render on Android #4126

Closed

s77rt reviewed Jun 13, 2023

View reviewed changes

s77rt mentioned this pull request Jun 13, 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

parasharrajat deleted the parasharrajat/quotedText branch November 20, 2023 13:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix inline code blocks #2527

fix inline code blocks #2527

parasharrajat commented Apr 22, 2021 •

edited

Loading

jasperhuangg Apr 22, 2021

parasharrajat Apr 22, 2021 •

edited

Loading

roryabraham left a comment

parasharrajat commented Apr 23, 2021

parasharrajat commented Apr 23, 2021

roryabraham Apr 23, 2021

parasharrajat Apr 23, 2021

parasharrajat Apr 23, 2021

jasperhuangg Apr 26, 2021

parasharrajat Apr 26, 2021 •

edited

Loading

parasharrajat commented Apr 23, 2021

parasharrajat commented Apr 23, 2021

jasperhuangg left a comment •

edited

Loading

parasharrajat commented Apr 26, 2021

parasharrajat commented Apr 26, 2021

roryabraham left a comment

parasharrajat commented Apr 27, 2021

parasharrajat commented Apr 29, 2021

jasperhuangg commented Apr 30, 2021

OSBotify commented Apr 30, 2021

OSBotify commented May 8, 2021

s77rt Jun 13, 2023

s77rt Jun 13, 2023

fix inline code blocks #2527

fix inline code blocks #2527

Conversation

parasharrajat commented Apr 22, 2021 • edited Loading

Details

Problem

How I fixed it?

Fixed Issues

Tests / QA Steps

Tested On

Screenshots

Web / Desktop

Mobile Web

Android

IOS

jasperhuangg Apr 22, 2021

Choose a reason for hiding this comment

parasharrajat Apr 22, 2021 • edited Loading

Choose a reason for hiding this comment

roryabraham left a comment

Choose a reason for hiding this comment

parasharrajat commented Apr 23, 2021

parasharrajat commented Apr 23, 2021

roryabraham Apr 23, 2021

Choose a reason for hiding this comment

parasharrajat Apr 23, 2021

Choose a reason for hiding this comment

parasharrajat Apr 23, 2021

Choose a reason for hiding this comment

jasperhuangg Apr 26, 2021

Choose a reason for hiding this comment

parasharrajat Apr 26, 2021 • edited Loading

Choose a reason for hiding this comment

parasharrajat commented Apr 23, 2021

parasharrajat commented Apr 23, 2021

jasperhuangg left a comment • edited Loading

Choose a reason for hiding this comment

parasharrajat commented Apr 26, 2021

parasharrajat commented Apr 26, 2021

roryabraham left a comment

Choose a reason for hiding this comment

parasharrajat commented Apr 27, 2021

parasharrajat commented Apr 29, 2021

jasperhuangg commented Apr 30, 2021

OSBotify commented Apr 30, 2021

OSBotify commented May 8, 2021

s77rt Jun 13, 2023

Choose a reason for hiding this comment

s77rt Jun 13, 2023

Choose a reason for hiding this comment

parasharrajat commented Apr 22, 2021 •

edited

Loading

parasharrajat Apr 22, 2021 •

edited

Loading

parasharrajat Apr 26, 2021 •

edited

Loading

jasperhuangg left a comment •

edited

Loading