[Payment 2/21] [$500] [Wave 8] [Ideal Nav] WS - When deleting a WS offline, it is not crossed out

[lanitochka17]

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.36-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: https://expensify.testrail.io/index.php?/tests/view/4278525&group_by=cases:section_id&group_order=asc&group_id=229065
Email or phone of affected tester (no customers): applausetester+emilio@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Access staging.new.expensify.com
2. Sign into a valid account
3. Tap on the Expensify logo on the top left > Expensify settings
4. Turn off internet
5. Tap on a 3 dot menu next to any WS and delete it while offline

Expected Result:

User expects the WS to be crossed out when deleted offline and then after returning online to disappear

Actual Result:

The WS instantly disappears

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

Bug6365190_1706899405908.WS_deleted_offline_is_not_crossed_out_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0119aa0aa444a2b68a
• Upwork Job ID: 1753523964291801088
• Last Price Increase: 2024-02-02
• Automatic offers:
• mkhutornyi | Contributor | 28146349

[melvin-bot]

Triggered auto assignment to @CortneyOfstad (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 @puneetlath (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[lanitochka17]

We think that this bug might be related to #wave6
CC @greg-schroeder

[greg-schroeder]

Hmm. This isn't really a Wave 6 specific issue, it's kind of just general to the app

[greg-schroeder]

It should obviously be fixed, but I don't think it needs to be tracked on the Wave 6 board

[CortneyOfstad]

@puneetlath Just let me know if there is anything you need from me as BZ! 👍

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0119aa0aa444a2b68a

[melvin-bot]

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

[CortneyOfstad]

Thanks @hayata-suenaga!

[mkhutornyi]

Proposal

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

When deleting a WS offline, it disappears from workspaces list

What is the root cause of that problem?

App/src/pages/workspace/WorkspacesListPage.tsx

Lines 172 to 187 in de633ac

	<OfflineWithFeedback
	key={`${item.title}_${index}`}
	pendingAction={item.pendingAction}
	errorRowStyles={styles.ph5}
	onClose={item.dismissError}
	errors={item.errors}
	>
	<PressableWithoutFeedback
	role={CONST.ROLE.BUTTON}
	accessibilityLabel="row"
	style={[styles.mh5, styles.mb3]}
	disabled={item.disabled}
	onPress={item.action}
	>
	{({hovered}) => (
	<WorkspacesListRow

In Ideal Navigation PR, MenuItem was replaced with WorkspaceListRow wrapped with PressableWithoutFeedback
If PressableWithoutFeedback is direct child view of OfflineWithFeedback and child of PressableWithoutFeedback is function, it fails to render in this logic:

App/src/components/OfflineWithFeedback.tsx

Lines 101 to 120 in de633ac

	const applyStrikeThrough = useCallback(
	(childrenProp: React.ReactNode): React.ReactNode => {
	const strikedThroughChildren = mapChildrenFlat(childrenProp, (child) => {
	if (!React.isValidElement(child)) {
	return child;
	}
	const props: StrikethroughProps = {
	style: StyleUtils.combineStyles(child.props.style, styles.offlineFeedback.deleted, styles.userSelectNone),
	};
	if (child.props.children) {
	props.children = applyStrikeThrough(child.props.children);
	}
	return React.cloneElement(child, props);
	});
	return strikedThroughChildren;
	},

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

1. change wrapping order to fix visibility issue

<PressableWithoutFeedback>
    <OfflineWithFeedback>
        <WorkspaceListRow />
    </OfflineWithFeedback>
</PressableWithoutFeedback>

2. rename rowStyles prop name to style in WorkspacesListRow to fix strikethrough issue. Because in applyStrikeThrough logic, style is added to view. Or add style at the end of style prop value in parent wrapper view of WorkspaceListRow.

3. hide three dots menu in WorkspaceListRow when pending delete
   There are several ways to do this. I prefer introducing new prop like shouldHideThreeDotsMenu={item.disabled}

[hayata-suenaga]

your approval comment was marked as outdated 😢 now I don't trust GH 😭

[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 📖

[mananjadhav]

Ohh yeah I marked it outdated because of this comment. Sorry I forgot to unhide it.

[hayata-suenaga]

ohhh you can actually make comment outdated manually didn't know that 😮

[mkhutornyi]

whether we should disable or hide the ThreeDotsMenu we'll have to confirm the behavior.

Can we confirm? I prefer hiding it as we don't have any pattern yet to disable 3 dots menu

[hayata-suenaga]

I believe we can just disable the dots 👍

[CortneyOfstad]

@mkhutornyi @mananjadhav — any update on the PR/review? Thanks!

[mkhutornyi]

PR was merged and hit staging

[mananjadhav]

@CortneyOfstad This should be ready for payout. Can you help with the payout summary?

Added a comment on the offending PR.

@hayata-suenaga @CortneyOfstad Can one of you confirm if a test exists for the offline feedback of WS deletion already? If not, then I think it makes sense to add a regression test here.

Copied the steps from the PR for regression test proposal.

1. Tap on the wrench on bottom tab
2. Turn off internet
3. Tap on 3 dots menu next to any workspace and delete it while offline
4. Verify that deleted workspace is crossed out
5. Tap on 3 dots menu again
6. Verify that 3 dots menu is disabled and not clickable

[CortneyOfstad]

@mananjadhav I did not see a regression test in relation to this, so I believe that creating one should be fine. If you're confident in those steps, I'll get regression test created 👍

Other Offline Mode tests were available in TestRail, but not removing a workspace, so would recommend adding here

Payment summary incoming!

[CortneyOfstad]

Payment Summary

@mkhutornyi (Contributor) — paid $500 via Upwork
@mananjadhav (C+) — to be paid $500 via NewDot

[mananjadhav]

Yes we used the same steps to test the PR and they worked fine.

[CortneyOfstad]

Perfect! Regression test has been created here

Also updated the title to reflect the payment date 👍

[CortneyOfstad]

@mananjadhav has a pay request been sent in NewDot yet?

[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.

[hayata-suenaga]

seems like the above deploy blocker is a known issue

[mkhutornyi]

yeah above Melvin comment is false alarm. That's because I referenced this issue here

[CortneyOfstad]

This is good to close! @mananjadhav if you have any trouble getting the NewDot request through because this is closed, feel free to reopen 👍

[JmillsExpensify]

$500 approved for @mananjadhav based on this summary.