[HOLD for payment 2024-03-06] [$250] Strikethrough line is white on links and emails

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.40-0
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

Action Performed:

1. Log in to New Expensify
2. Nagivate to any chat
3. Send any email or link with strikethrough markdown

Expected Result:

Strikethrough line should match the color of links and emails.

Actual Result:

Strikethrough line is white.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6376813_1707754899905.2024-02-12_17-20-25.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0190109e65c0e5c4d0
• Upwork Job ID: 1757082391060185088
• Last Price Increase: 2024-02-12
• Automatic offers:
• mollfpr | Contributor | 0
• mkhutornyi | Contributor | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0190109e65c0e5c4d0

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary (External)

[melvin-bot]

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @francoisl (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[francoisl]

Can reproduce, but no need to block on this, I'll demote. Also going to update the price as it's most likely a simple change.

[melvin-bot]

Upwork job price has been updated to $250

[Krishna2323]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Strikethrough line is white on links and emails

What is the root cause of that problem?

We already have text-decoration styles applied to a tag, so the styles from the del tag isn't applied correctly because the text-decoration-color: rgb(90, 176, 255);.

What changes do you think we should make in order to solve the problem?

To apply both underline & line-through we need to wrap the TNodeChildrenRenderer with Text which will apply the styles for the line-through .

Steps:

1. Create a new HTML engine util function named isChildOfDel which will identify if the anchor is inside del tag or not.

function isChildOfDel(tnode: TNode): boolean {
    return isChildOfNode(tnode, (node) => node.domNode?.name !== undefined && node.domNode.name.toLowerCase() === 'del');
}

2. Check if the anchor is inside del tag or not.

const isInsideDel = HTMLEngineUtils.isChildOfDel(tnode);

3. Wrap the TNodeChildrenRenderer with Text with correct textDecorationLine style, we also need to flatten style from prop.const flattenStyles = StyleSheet.flatten(style as TextStyle);

<Text style={[flattenStyles, {textDecorationLine: isInsideDel ? 'line-through' : undefined}]}>
    <TNodeChildrenRenderer tnode={tnode} />
</Text>

Result

strikeThroughColor.mp4

Alternative

Or we can only show the line-through by overriding the styles if it is inside del .

style={[parentStyle, styles.textUnderlinePositionUnder, styles.textDecorationSkipInkNone, style, isInsideDel && {textDecorationLine: 'line-through'}]}

[Krishna2323]

@francoisl, can you pls increase the bounty🙏🏻, this isn't a simple change, It took me good amount of time to figure out the root cause and to find a solution. Even if my solution is not valid then also I think we can't find a simple change.

[situchan]

@Krishna2323 this was deploy blocker. Are you able to find offending PR?
I suspect this came from expensify-common while working on live markdown regression fixes

[Krishna2323]

No, I couldn't find the offending PR.

[mkhutornyi]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Strikethrough line is white on links and emails

What is the root cause of that problem?

regression from this line:
https://github.com/Expensify/App/pull/35462/files#diff-4935ec01feca8054d8a080f44501a37a1cf1398ffd19e0b46d3551a56e5ba313R66

- style={{...props.style, ...parentStyle, ...styles.textUnderlinePositionUnder, ...styles.textDecorationSkipInkNone}}
+ style={[parentStyle, styles.textUnderlinePositionUnder, styles.textDecorationSkipInkNone, style]}

So style overrides other custom styles by putting at the end

What changes do you think we should make in order to solve the problem?

fix that order so put style at the beginning as before

style={[style, parentStyle, styles.textUnderlinePositionUnder, styles.textDecorationSkipInkNone]}

[Krishna2323]

I think we can't find a simple change

Lol 😂, I thought we need both, underline & strike-through at the same time.

[jjcoffee]

@garrettmknight FYI @ntdiary is OOO until 18th Feb, I can take this one to keep things moving if you want 🙇

[situchan]

I think @mollfpr should handle this as he reviewed offending PR

[melvin-bot]

📣 @mkhutornyi 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[mollfpr]

@mkhutornyi Let me know when the PR is ready, thank you!

[mkhutornyi]

PR will be ready by end of week

[francoisl]

(Commenting to remove the Overdue label, see comment just above about the PR)

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[mollfpr]

@garrettmknight Could you add the payment date? The blocker issue is closed, so there's no regression on the PR (for now).

[garrettmknight]

Payment Summary

• Contributor: @mkhutornyi $250 paid via Upwork
• Reviewer:@mollfpr $250

[mollfpr]

@garrettmknight Requested in NewDot! I'll complete the checklist soon.

[garrettmknight]

Cool, dropping to weekly.

[mollfpr]

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:

#35462

[@mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

https://github.com/Expensify/App/pull/35462/files#r1518590135

[@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

I think the regression step should be good and we also have a checklist to check the generic component.

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

We can a case for strikethrough an link/email and verify the strikethrough color match the link color. https://expensify.testrail.io/index.php?/cases/view/2950395

[garrettmknight]

TestRail update in - just up to you to submit for payment now!

[JmillsExpensify]

$250 approved for @mollfpr based on summary.