[HOLD for payment 2023-09-29] [$1000] Offline: Deleting edited inlinecode block and codeblock results in extra hyphen (strikethrough)

[kavimuru]

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

---

Action Performed:

1. Send inline codeblock and edit it. (abc )
2. Send Codeblock and edit it.
3. Go offline
4. Delete the edited codeblocks

Expected Result:

Strikethrough should be applied only to text. There should not be any extra hyphens.

Actual Result:

Unexpected extra hyphens

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.2.99-4
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @sobitneupane
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681289647224179

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c51c938ba2ca45e8
• Upwork Job ID: 1648053225730277376
• Last Price Increase: 2023-05-08

[MelvinBot]

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

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[isabelastisser]

I don't have the capacity to work on this today, so I will prioritize it next week.

[isabelastisser]

I will prioritize this today.

[isabelastisser]

Reviewing this now.

[isabelastisser]

I was able to reproduce it:

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~01c51c938ba2ca45e8

[MelvinBot]

Current assignee @isabelastisser is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

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

[MelvinBot]

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

[ahmedGaber93]

i think it duplicated #17181

[AndrewGable]

Can you confirm and close if so @isabelastisser? Thanks!

[sobitneupane]

i think it duplicated #17181

No, it is not. It's a different issue. The other issue is about the alignment of strikethrough.

The issue here is an extra hyphen present before (edited) in inline codeblock and codeblock:

[bernhardoj]

Proposal

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

There is an extra strikethrough on a pending delete text. It's not only happen to code block, but to all text.

What is the root cause of that problem?

The space between (edited) and the text is a whitespace character, so it also get the strikethrough style.

App/src/components/HTMLEngineProvider/HTMLRenderers/EditedRenderer.js

Lines 18 to 27 in 8864416

	<Text
	// eslint-disable-next-line react/jsx-props-no-spreading
	{...defaultRendererProps}
	fontSize={variables.fontSizeSmall}
	color={themeColors.textSupporting}
	>
	{/* Native devices do not support margin between nested text */}
	<Text style={styles.w1}>{' '}</Text>
	{props.translate('reportActionCompose.edited')}
	</Text>

App/src/pages/home/report/ReportActionItemFragment.js

Lines 133 to 138 in 8864416

	<Text
	fontSize={variables.fontSizeSmall}
	color={themeColors.textSupporting}
	>
	{` ${props.translate('reportActionCompose.edited')}`}
	</Text>

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

Instead of using space character, we can replace it with an empty View with the same style, which is styles.w1 (width: 4).

[dukenv0307]

Proposal

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

Unexpected extra hyphens in the edited text when deleting edited codeblock/inline codeblock message in offline mode.

What is the root cause of that problem?

In here

App/src/components/HTMLEngineProvider/HTMLRenderers/EditedRenderer.js

Line 32 in 3ce8e82

{' '}

we're adding a space at the beginning of the edited label to add some spacing with the "edited" message.

In here

App/src/components/HTMLEngineProvider/HTMLRenderers/EditedRenderer.js

Line 25 in 3ce8e82

style={[editedLabelStyles, isPendingDelete && styles.offlineFeedback.deleted]}

, we're adding the strike through style if the message is pending deleted.

The empty space will get the strike through CSS style as well so it shows as such.

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

We need to wrap the (edited) here

App/src/components/HTMLEngineProvider/HTMLRenderers/EditedRenderer.js

Line 34 in 3ce8e82

{props.translate('reportActionCompose.edited')}

in a Text component and move the isPendingDelete && styles.offlineFeedback.deleted style there instead, so it will only apply for the (edited) text, not the spacing space.

That's for HTML message, if we don't want the strike-through to apply for the space in case of text message as well, then we can apply the same approach here

App/src/pages/home/report/ReportActionItemFragment.js

Line 134 in 3ce8e82

{' '}

(only apply the ...props.style to the relevant text part rather than to the whole container component which includes the spacing space).

What alternative solutions did you explore? (Optional)

NA

[isabelastisser]

Not overdue; still accepting proposals.

[MelvinBot]

@AndrewGable, @isabelastisser, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Fix/17365 nostrikethrough solution #26176

If no regressions arise, payment will be issued on 2023-09-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @aimane-chnaif requires payment
• @sobitneupane does not require payment (Eligible for Manual Requests)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [@aimane-chnaif] 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:
• [@aimane-chnaif] 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:
• [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [@aimane-chnaif] 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.
• [@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[isabelastisser]

@aimane-chnaif requires payment: I sent the proposal in Upwork now.
@sobitneupane does not require payment (Eligible for Manual Requests) - the payment will be processed via newdot.

[isabelastisser]

@aimane-chnaif, please complete the checklist above so we can close the issue. Thanks!

[aimane-chnaif]

This bug existed for a long time and there were back and forth changes related to (edited) strikethrough so no sense of pinpointing offending PR.
As this is minor UI bug which doesn't affect app logic and also rare case (edit + offline + delete + codeblock), not worth of adding regression test.

[isabelastisser]

Thanks @aimane-chnaif ! Closing.

[sobitneupane]

Requested payment of $250 as reporter of the bug in newDot.

[JmillsExpensify]

Re-opening because I need a payment summary for this issue.

[trjExpensify]

Issue created prior to Aug 30th, so old bug bounty levels apply for this issue.

• @sobitneupane - $250 for the bug report
• @koko57 is from Callstack and fixed the bug, payment will be handled separately. For tracking purposes though, I'm going to assign you to this issue though as that step was missed. Let's make sure that gets actioned on going forward!
• @aimane-chnaif $1,000 for the C+ review of the PR. No #urgency bonus to be applied. Despite a merge freeze for ~1 week in between, the proposal was given the green light on Aug 29th and approved on Sept 18th which is more than 3 business days despite that.

[JmillsExpensify]

$250 payment approved for @sobitneupane based on summary above.

[trjExpensify]

Cool, thanks! @aimane-chnaif were you paid for this via Upwork as per the checkmarks here?

[aimane-chnaif]

yes, got 1k. Let's close

[trjExpensify]

Perf, closing!