[HOLD for payment 2021-12-13] URL link is mangled up with the rest of the text

[sketchydroide]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Wrote Chronos 10am update on desktop
2. Added GH links to the update
3. submitted
4. opened conversation on mobile iOS
5. Links are on top of the other text

Expected Result:

The links should take their own space

Actual Result:

Link is on top of the normal text

Workaround:

N/A

Platform:

Where is this issue occurring?

• iOS
• Android (not sure, I haven't checked)

Version Number:
V1.0.86-4
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @michaelhaxhiu (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[michaelhaxhiu]

@mallenexpensify I have searched titles with various naming conventions, but can't find another GH that looks like this one. Before I push this through, I'd love a buddy check from you 🙏 . Have you come across this before? I feel like I've seen it once before but can't tell if I'm crazy or not.

[sketchydroide]

This happened when I added a url to my 10ams

[mallenexpensify]

@michaelhaxhiu yup! Saw for the first time last night or this morn. iOS v1.0.90-4

I'll move it along since you're likely already off Hax

[MelvinBot]

Triggered auto assignment to @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[michaelhaxhiu]

@sketchydroide sorry for the confusion, I agree this is a real problem (I reproduced it myself 😄) but was trying to make sure it wasn't a duplicate. We are all set, thanks Matt!

[MelvinBot]

@johnmlee101 Huh... This is 4 days overdue. Who can take care of this?

[johnmlee101]

Oh! I think everything above looks good and should be good to toss this out for external contributions. I believe its contained to just long links if I'm not mistaken?

[MelvinBot]

Triggered auto assignment to @arielgreen (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[arielgreen]

Posted to Upwork here: https://www.upwork.com/jobs/~0113b0cbb1016c18de
https://www.upwork.com/jobs/~01b36da7505c041642

[meetmangukiya]

I am able to fix this by commenting out the lineHeight css in baseFontStyle

App/src/styles/styles.js

Line 2133 in 1f332a6

lineHeight: variables.fontSizeNormalHeight,

not sure why. We could do that unless we have any reason to hardcode to 20 and not use the default lineHeight.

A little more context: the problem is with rendering the html which we are doing with RenderHTMLSource. This is also affected by TRenderEngineProvider and RenderHTMLConfigProvider

App/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js

Lines 237 to 254 in f658da2

	<TRenderEngineProvider
	customHTMLElementModels={customHTMLElementModels}
	baseStyle={webViewStyles.baseFontStyle}
	tagsStyles={webViewStyles.tagStyles}
	enableCSSInlineProcessing={false}
	dangerouslyDisableWhitespaceCollapsing={false}
	systemFonts={EXTRA_FONTS}
	>
	<RenderHTMLConfigProvider
	defaultTextProps={defaultTextProps}
	defaultViewProps={defaultViewProps}
	renderers={renderers}
	renderersProps={renderersProps}
	computeEmbeddedMaxWidth={computeEmbeddedMaxWidth}
	>
	{children}
	</RenderHTMLConfigProvider>
	</TRenderEngineProvider>

. I tried commenting out different props and trying to isolate the problem, and commenting out the lineHeight in baseStyle was when it started working.

Proposal

Remove lineHeight styling from baseFontStyle.

App/src/styles/styles.js

Line 2133 in 1f332a6

lineHeight: variables.fontSizeNormalHeight,

[johnmlee101]

I'm curious @meetmangukiya do you know why commenting that out fixes the issue? Is there another way for us to make sure word wrapping with urls works well?

[parasharrajat]

There is more to it. Basically, BoxModel of RenderHTMLEngine is not compatible with this design so it is a technical limitation. #4733 is also caused by the same issue. I am trying to find a solution to it.

[arielgreen]

@jboniface reassigning, I'm off for mat leave!

[parasharrajat]

Can I submit the PR or.....?

[jboniface]

@parasharrajat yeah i think so; we didn't have you hired yet, did we? here's a link to the UW job just in case https://www.upwork.com/jobs/~0198867f3b0915c30b

[MelvinBot]

Current assignee @johnmlee101 is eligible for the Exported assigner, not assigning anyone new.

[MelvinBot]

📣 @parasharrajat You have been assigned to this job by @jboniface!
Please apply to this job in Upwork and let us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[parasharrajat]

So, I am officially hired now. but I have a concern to share before I move to PR. This PR will also fix #5572. I am eligible for the payment of that issue as well. If not, I would like to request a bonus here not because it is too much work but I have been following this issue for quite a time now. There have been multiple holds and I have spent a lot of time finding a solution without the possibility that I will be hired.

Let me know what do you think @jboniface?

[jboniface]

Considering you're hired on both jobs, you'd get paid for both jobs, right? We wouldn't invalidate one just because the same PR solved both issues (although we might not have filed both jobs if we knew this ahead of time). Do you think we should pay out more than the reward for both the jobs? In my opinion, the other issue was on n6-hold and you had a solution during the hold, so that one should be 500. This one would be 250. So 750 for one PR -- that seems like a fair deal. What do you think?

[mountiny]

@parasharrajat I will make sure you are also hired for the other issue as well. Then just make sure to link both of the issues in the PR 🙌

[parasharrajat]

If I am hired for other as well. Then it's fine.

Thanks @mountiny.

[mountiny]

Perfect! No worries and thank you for digging into this and the related issue 🙌

[jboniface]

on staging

[botify]

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

• Fix URL rendering #6392

If no regressions arise, payment will be issued on 2021-12-13. 🎊

[jboniface]

@parasharrajat will pay when you accept, sorry I missed the proposal earlier

[jboniface]

paid