[HOLD App PR #40839] Display explanatory texts on the tags screen - #40811
Conversation
|
I'm facing this TypeScript error where the 
|
|
Assigning @s77rt here as they have the context behind this PR. |
|
@hayata-suenaga Add App/src/pages/workspace/withPolicy.tsx Lines 31 to 51 in 7ea2b6c |
|
I'm still seeing a ts error. Based on other HOC we should extend the props. Here this should work export default function <TProps extends WithPolicyConnectionsProps>(WrappedComponent: ComponentType<TProps>) {
function WithPolicyConnections(props: TProps) {
const {isOffline} = useNetwork();
const [hasConnectionsDataBeenFetched, {status}] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_HAS_CONNECTIONS_DATA_BEEN_FETCHED}${props.policy?.id ?? '0'}`, {
initWithStoredValues: false,
});
useEffect(() => {
// When the accounting feature is not enabled, or if the connections data already exists,
// there is no need to fetch the connections data.
if (!props.policy || !props.policy.areConnectionsEnabled || !!hasConnectionsDataBeenFetched || !!props.policy.connections) {
return;
}
openPolicyAccountingPage(props.policy.id);
}, [hasConnectionsDataBeenFetched, props.policy, isOffline]);
if (!props.policy || status === 'loading' || !hasConnectionsDataBeenFetched) {
return (
<FullPageOfflineBlockingView>
<FullScreenLoadingIndicator />
</FullPageOfflineBlockingView>
);
}
return (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
/>
);
}
return withPolicy(WithPolicyConnections);
} |
|
Looks you made changes to the wrong file. It should be |
|
@cristipaval looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
how in the world did I merge this? 😨 I'm seeing this PR for the first time. It must have been something automated. I merged the other PR related to this |
|
@cristipaval Can you please create a revert for this one? |
|
I discussed this internally, and it seems that @hayata-suenaga has forked the other PR from this one, which is why GH automatically marked this one as merged. All these commits are already reverted in the other PR, see its commits list to better understand. So we are fine. @hayata-suenaga will open another PR. |
|
@hayata-suenaga, suggestion: you could manually assign @s77rt as a reviewer to the new PR you will open, given that he already has context. |
Details
This PR
Fixed Issues
$ #40798
PROPOSAL: N/A
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
MacOS: Desktop