Fix: blank space below referral banner on manual expense participants page#94754
Fix: blank space below referral banner on manual expense participants page#94754MelvinBot wants to merge 4 commits into
Conversation
… IOU participants page Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
@MelvinBot Replace all |
|
@Eskalifer1 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariOld flow: Screen.Recording.2026-06-27.at.00.28.52.movNew flow: Screen.Recording.2026-06-27.at.00.30.20.mov |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f36f22b8ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // The participant list's SelectionList footer (referral banner / Next button) owns the bottom | ||
| // safe area inset via `addBottomSafeAreaPadding`. Opt into edge-to-edge so the screen container | ||
| // doesn't also reserve the inset, which would double-pad and leave a blank gap below the banner. | ||
| enableEdgeToEdgeBottomSafeAreaPadding |
There was a problem hiding this comment.
In the dismissed-banner/no-selection state, ParticipantSearchResults passes no footerContent, so the list only adds bottom safe-area padding; opting the wrapper into edge-to-edge mode also makes ScreenWrapper render the mobile offline indicator as a sticky absolute element. If the user is offline in this state, the offline bar overlays the bottom of the participant list, whereas the previous legacy wrapper laid the indicator out below the content. Please either keep the non-sticky offline behavior for this screen or add an offline-indicator offset when the footer is absent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I checked this — the concern is valid in mechanism, though it's a narrow cosmetic edge case rather than a functional break.
What's real: Opting StepScreenWrapper into edge-to-edge mode flips the mobile offline indicator from in-flow layout to an absolutely-positioned sticky bar (position: absolute; bottom: 0), via ScreenWrapper/index.tsx#L175 → ScreenWrapperOfflineIndicators.tsx#L95 (styles.stickToBottom). When a footer is present, FixedFooter reserves OFFLINE_INDICATOR_HEIGHT so content clears the sticky bar. But in the no-footer state (banner dismissed + no selection, ParticipantSearchResults.tsx#L418) the list's bottom padding comes from safeAreaPaddingBottomStyle, which reserves only the inset and not the indicator height (BaseSelectionListWithSections.tsx#L89). So when offline in that exact state, the sticky bar overlays the last ~25px of the list — whereas legacy mode laid it out below content.
Why the bot caught it: The sibling NewChatPage (already edge-to-edge, same dismissed-banner pattern) avoids this by not rendering the sticky indicator on its wrapper — NewChatPage/index.tsx#L476-L477 passes disableOfflineIndicatorSafeAreaPadding + shouldShowOfflineIndicator={false} (its parent owns the indicator). This page keeps StepScreenWrapper's default shouldShowOfflineIndicator = true, so it does render the sticky bar and inherits the overlap.
Severity: Only triggers when all of: mobile + offline + banner dismissed + zero participants selected. The bar is translucent and sits at the bottom of a scrollable list, so the impact is cosmetic. Real regression vs. legacy, but minor.
Suggested fix (lowest risk): Mirror NewChatPage — pass shouldShowOfflineIndicator={false} (and optionally disableOfflineIndicatorSafeAreaPadding) through StepScreenWrapper for this screen, since the SelectionList already owns the bottom padding. The broader alternative (making the no-footer branch of BaseSelectionListWithSections reserve OFFLINE_INDICATOR_HEIGHT when offline) would touch all SelectionList consumers and is riskier.
Worth a quick Android emulator repro (offline + dismissed banner + no selection) to confirm the visual impact before deciding whether to address it in this PR or fast-follow.
Want me to implement the shouldShowOfflineIndicator={false} fix? Reply @MelvinBot implement this and I'll push it to the branch.
There was a problem hiding this comment.
Implemented in 104b4cc.
Mirrored NewChatPage exactly for this screen:
IOURequestStepParticipants.tsxnow passesshouldShowOfflineIndicator={false}+disableOfflineIndicatorSafeAreaPadding, so theSelectionListis the sole owner of the bottom padding and the sticky bar can no longer overlap the last rows in the no-footer state.StepScreenWrapperdidn't forwarddisableOfflineIndicatorSafeAreaPadding, so I added it as an optional pass-through prop (StepScreenWrapper.tsx). It defaults toundefined, so every otherStepScreenWrapperusage is unchanged.
I added disableOfflineIndicatorSafeAreaPadding in addition to shouldShowOfflineIndicator={false} so the footer doesn't reserve OFFLINE_INDICATOR_HEIGHT for an indicator that's no longer rendered (which would otherwise reintroduce a small gap below the banner while offline).
Verification: prettier, npm run typecheck-tsgo, lint, and the React Compiler compliance check all pass. This is a layout-only change; a quick Android emulator repro (offline + dismissed banner + no selection) is still worth doing to confirm the sticky bar no longer overlaps the list.
There was a problem hiding this comment.
@MelvinBot Your fix doesn't work, please revert all changes in your PR and apply only my changes below. It should work
--- a/src/pages/iou/request/ParticipantSearchResults.tsx
+++ b/src/pages/iou/request/ParticipantSearchResults.tsx
@@ -1,5 +1,5 @@
import lodashPick from 'lodash/pick';
-import React, {useEffect} from 'react';
+import React, {useContext, useEffect} from 'react';
import type {Ref} from 'react';
import type {GestureResponderEvent} from 'react-native';
import {RESULTS} from 'react-native-permissions';
@@ -7,6 +7,7 @@ import ContactPermissionModal from '@components/ContactPermissionModal';
import EmptySelectionListContent from '@components/EmptySelectionListContent';
import MenuItem from '@components/MenuItem';
import {usePersonalDetails} from '@components/OnyxListItemProvider';
+import ScreenWrapperStatusContext from '@components/ScreenWrapper/ScreenWrapperStatusContext';
import InviteMemberListItem from '@components/SelectionList/ListItem/InviteMemberListItem';
import SelectionListWithSections from '@components/SelectionList/SelectionListWithSections';
import type {Section, SelectionListWithSectionsHandle} from '@components/SelectionList/SelectionListWithSections/types';
@@ -147,6 +148,12 @@ function ParticipantSearchResults({
const reportAttributesDerived = useReportAttributes();
const privateIsArchivedMap = usePrivateIsArchivedMap();
+ // When the surrounding ScreenWrapper runs in edge-to-edge mode (e.g. the new manual expense flow's ParticipantPicker),
+ // it does not apply bottom safe area padding itself, so the fixed footer must consume it. Otherwise the footer
+ // (referral banner / Next button) renders behind the system navigation bar.
+ const screenWrapperStatusContext = useContext(ScreenWrapperStatusContext);
+ const addBottomSafeAreaPadding = !(screenWrapperStatusContext?.isSafeAreaBottomPaddingApplied ?? false);
+
// Policy and billing data — owned here, used for getValidOptionsConfig and billing gate in onSelectRow
const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID);
const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
@@ -514,6 +521,7 @@ function ParticipantSearchResults({
/>
}
footerContent={footerContent}
+ addBottomSafeAreaPadding={addBottomSafeAreaPadding}
listEmptyContent={EmptySelectionListContentWithPermission}
shouldShowLoadingPlaceholder={shouldShowLoadingPlaceholder}
shouldShowTextInput
|
@Eskalifer1 Sorry, please ignore this PR. It is my regression, I will review it |
|
@MelvinBot Could you please check this comment? #94754 (comment) |
… overlap Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
@Pujan92 @puneetlath One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
…m ScreenWrapperStatusContext in ParticipantSearchResults Reverts the changes to IOURequestStepParticipants and StepScreenWrapper and instead lets the participant SelectionList footer own the bottom safe area inset by reading isSafeAreaBottomPaddingApplied from ScreenWrapperStatusContext. Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
@MelvinBot Test failed but it doesn't related to our changes, let's re-run the test (if you don't have permission, let's push another commit to delete the code comment so that the test is re-run |
Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
2 participants
Explanation of Change
The manual-expense participant ("To") page showed a blank gap below the referral banner on Android, caused by double bottom safe-area padding.
#94539 added
addBottomSafeAreaPaddingto theSelectionListWithSectionson this page so the banner'sFixedFooterreserves the bottom inset. But this page is wrapped byStepScreenWrapper, which rendersScreenWrapperin legacy mode (noenableEdgeToEdgeBottomSafeAreaPadding), so the screen container already reserves the bottom inset. With both the container and the footer applying the inset, the result is an inset-sized blank gap under the banner.The sibling
NewChatPagerenders the same banner + prop correctly because itsScreenWrapperis edge-to-edge (includeSafeAreaPaddingBottombecomesfalse), so theSelectionListis the single owner of the bottom inset.This PR makes the IOU participants screen match that pattern: it adds an optional
enableEdgeToEdgeBottomSafeAreaPaddingprop toStepScreenWrapperand sets it only onIOURequestStepParticipants. The container stops reserving the inset and theSelectionListowns it — via theFixedFooterwhen a footer is present, or via the list content-container padding when it isn't. Scoping the prop to this one screen leaves all otherStepScreenWrapperusages on the legacy path unchanged (zero blast radius).Fixed Issues
$ #94744
PROPOSAL: #94744 (comment)
Tests
// TODO: The human co-author must fill out the tests they ran before marking this PR as "ready for review".
Suggested verification (Android device with a 3-button navigation bar):
Offline tests
N/A — layout-only change.
QA Steps
// TODO: The human co-author must fill out the QA steps before marking this PR as "ready for review".
Suggested QA (Android device with the navigation set to bar, not gestures):
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari