[HOLD for payment 2024-06-13] [$250] Workflows - Offline indicator is present below the last option instead of page bottom

[m-natarajan]

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

---

Version Number: 1.4.68-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
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. Launch New Expensify app.
2. Go offline.
3. Go to Account settings > Workspaces > any workspace.
4. Go to Workflows.
5. Tap Submission frequency.

Expected Result:

Offline indicator will be present in the bottom of the page.

Actual Result:

Offline indicator is present below the last option.

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

Bug6466547_1714481116830.RPReplay_Final1714480895.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b48de547df48461b
• Upwork Job ID: 1787886904799145984
• Last Price Increase: 2024-05-07
• Automatic offers:
• hoangzinh | Reviewer | 0
• Krishna2323 | Contributor | 0

Issue Owner

Current Issue Owner: @puneetlath / @muttmuure

[melvin-bot]

Triggered auto assignment to @muttmuure (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[m-natarajan]

@muttmuure FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[neonbhai]

Proposal

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

Offline indicator is present below the last option instead of page bottom

What is the root cause of that problem?

We don't use the full height of the page in WorkspaceAutoReportingFrequencyPage

We are also missing the ability to scroll the page:

Screen.Recording.2024-04-30.at.7.16.58.PM.mov

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

We will wrap the contents of the page with ScrollView passing the style flexGrow1 to fix both the issues

<ScrollView
    style={styles.pt0}
    contentContainerStyle={[styles.flexGrow1]}
>
    ...
</ScrollView>

Result

This works perfectly:

Screen.Recording.2024-04-30.at.7.16.07.PM.mov

The offline indicator is shown at the bottom and the page is now scrollable.

[allgandalf]

App/src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx

Lines 125 to 128 in b62bbbb

	<ScreenWrapper
	includeSafeAreaPaddingBottom={false}
	testID={WorkspaceAutoReportingFrequencyPage.displayName}
	>

Proposal

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

Offline indicator is not shown on the bottom of the page

What is the root cause of that problem?

We do not pass the property offlineIndicatorStyle to the ScreenWrapper, hence it takes default value and shows the indicator right after the list ends.

App/src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx

Lines 125 to 128 in b62bbbb

	<ScreenWrapper
	includeSafeAreaPaddingBottom={false}
	testID={WorkspaceAutoReportingFrequencyPage.displayName}
	>

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

We need to pass offlineIndicatorStyle={styles.mtAuto} to the Screenwrapper, similar approach was used in this PR as well.

Result

mweb iOS

[Krishna2323]

Proposal

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

Workflows - Offline indicator is present below the last option instead of page bottom

What is the root cause of that problem?

There are several issue on the WorkspaceAutoReportingFrequencyPage which should be solved.

1. The offline indicator position
2. Selected option is not focused initially
3. Keyboard navigation isn't working like other pages, e.g. Language settings page, Theme selection page

The cause of this is using FlatList instead of SelectionList. Because we need to add a sibling component besides the RadioListItem we are using FlatList.

App/src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx

Lines 109 to 118 in 35772b2

	const renderItem = ({item}: {item: WorkspaceAutoReportingFrequencyPageItem}) => (
	<>
	<RadioListItem
	item={item}
	onSelectRow={() => onSelectAutoReportingFrequency(item)}
	showTooltip={false}
	/>
	{isMonthlyFrequency && item.keyForList === CONST.POLICY.AUTO_REPORTING_FREQUENCIES.MONTHLY ? monthlyFrequencyDetails() : null}
	</>
	);

App/src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx

Lines 144 to 148 in 35772b2

	<FlatList
	data={autoReportingFrequencyItems}
	renderItem={renderItem}
	keyExtractor={(item: WorkspaceAutoReportingFrequencyPageItem) => item.text}
	/>

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

If we want to solve the offline indicator issue then we can style={styles.flex1} & contentContainerStyle={styles.flex1} to OfflineWithFeedback which wraps the FlatList. Also, we can pass isFocused={item.isSelected} to RadioListItem . This will make the selected option focused.

What alternative solutions did you explore? (Optional)

If we want to solve all the issues which, we can accept a new prop in ListItem type called itemBottom and then inside BaseSelectionList, when rendering item we will render the {item.itemBottom && item.itemBottom} besides ListItem. After that we can use SelectionList instead of FlatList inside WorkspaceAutoReportingFrequencyPage. And in autoReportingFrequencyItems data we will add itemBottom. We will also pass initiallyFocusedOptionKey={policy?.autoReportingFrequency} to SelectionList.

PS: minor changes can be made during the PR phase.

Result main solution

main_offline_indicator_submission_frequency.mp4

Result alternative solution

offline_indicator_submission_frequency.mp4

[Krishna2323]

Proposal Updated

• Minor edit in main solution
• Result videos added

[melvin-bot]

@muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[melvin-bot]

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

[muttmuure]

@hoangzinh there are some proposals to take a look at

[hoangzinh]

@neonbhai, @GandalfGwaihir, @Krishna2323 Thanks for proposal, everyone.

@Krishna2323 proposal looks good to me. I think we should use SelectionList to fix all issues in that page.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[allgandalf]

I think we should use SelectionList to fix all issues in that page.

Just for my info is it always the case that,

okay this is the issue, give solution to it, and then find some more bugs related to the same component and fix those too and your proposal will get priority ? @hoangzinh @stitesExpensify ? 🤔

P.S.: Just to be clear no conflict with the chosen proposal, question Is just for my info !

[hoangzinh]

Not really @GandalfGwaihir. When we select a solution, it is based on multiple factors, also based on each reviewer. For your case, it's because your RCA is not convincing me if we're solving the root cause or just a workaround solution. The question for your RCA is why on other pages, we don't pass offlineIndicatorStyle={styles.mtAuto} to the Screenwrapper but it still works?

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @Krishna2323 🎉 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 📖

[stitesExpensify]

Assigned Krishna, I'm going to unassign myself because I am about to go OOO for 2 weeks. @muttmuure can find a replacement by removing and re-adding external

[Krishna2323]

@hoangzinh, PR ready for review.

[melvin-bot]

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

[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.4.79-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: Workflows - Offline indicator is present below the last option instead of page bottom. #42019

If no regressions arise, payment will be issued on 2024-06-13. 🎊

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

• @hoangzinh requires payment automatic offer (Reviewer)
• @Krishna2323 requires payment automatic offer (Contributor)

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

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

[Krishna2323]

@puneetlath, bump for payments process.

[hoangzinh]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: [Simplified Collect][Workflows] Create AutoReporting page and commands #37213
• 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: [Simplified Collect][Workflows] Create AutoReporting page and commands #37213 (comment)
• 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: N/A
• Determine if we should create a regression test for this bug. Nope for me, it's just a small UI improvement.

[puneetlath]

@muttmuure is the BZ here. He'll handle payment.

[muttmuure]

Paid