Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix copilot bugs #48517

Merged
merged 22 commits into from
Sep 18, 2024
Merged

Fix copilot bugs #48517

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9221a03
fix overflow name
rushatgabhane Sep 4, 2024
84d33b4
Merge branch 'main' of github.com:rushatgabhane/exfy into fix-copilot…
rushatgabhane Sep 8, 2024
9c22426
preserve isloading
rushatgabhane Sep 8, 2024
7f07f36
show loading screen for subscription page
rushatgabhane Sep 8, 2024
f550dff
disable feedback when cannot switch account
rushatgabhane Sep 8, 2024
2b746dc
fix avatar position when upload error
rushatgabhane Sep 8, 2024
ad3bf76
show avatar skeleton when loading
rushatgabhane Sep 8, 2024
8d27ce0
rm IS_LOADING_REPORT_DATA
rushatgabhane Sep 8, 2024
6ee983f
reset error when source / avatar id changes
rushatgabhane Sep 8, 2024
c82df42
Merge branch 'main' into fix-copilot-bugs
rushatgabhane Sep 16, 2024
4033c2a
Merge branch 'main' of github.com:rushatgabhane/exfy into fix-copilot…
rushatgabhane Sep 16, 2024
0618191
add center style avatar
rushatgabhane Sep 16, 2024
ca0e5d4
fix #48965
rushatgabhane Sep 16, 2024
b1365f4
add fallback avatar. fix #48966
rushatgabhane Sep 16, 2024
ae039e1
fix #48967 show no results found
rushatgabhane Sep 16, 2024
412544c
fix #48972
rushatgabhane Sep 16, 2024
cdbf52f
fix center avatar on center
rushatgabhane Sep 16, 2024
186dfde
Update src/pages/settings/Security/AddDelegate/SelectDelegateRolePage…
rushatgabhane Sep 16, 2024
81e49dd
undo
rushatgabhane Sep 16, 2024
ef39885
migrate to useOnyx
rushatgabhane Sep 16, 2024
ef72f6e
Update src/pages/settings/Security/AddDelegate/SelectDelegateRolePage…
rushatgabhane Sep 16, 2024
747372d
fix lint
rushatgabhane Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/components/AccountSwitcher.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ function AccountSwitcher() {
}}
ref={buttonRef}
interactive={canSwitchAccounts}
pressDimmingValue={canSwitchAccounts ? undefined : 1}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed fix #48253

fix4.mov

wrapperStyle={[styles.flexGrow1, styles.flex1, styles.mnw0, styles.justifyContentCenter]}
>
<View style={[styles.flexRow, styles.gap3]}>
Expand All @@ -145,7 +146,7 @@ function AccountSwitcher() {
<View style={[styles.flexRow, styles.gap1]}>
<Text
numberOfLines={1}
style={[styles.textBold, styles.textLarge]}
style={[styles.textBold, styles.textLarge, styles.flexShrink1]}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please link fixed issue per each change (github comment) so I can have context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkhutornyi you can look at the commits for that - https://github.com/Expensify/App/pull/48517/commits

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed fix #48261 on native

fix1

>
{currentUserPersonalDetails?.displayName}
</Text>
Expand Down
13 changes: 9 additions & 4 deletions src/components/AvatarSkeleton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
import React from 'react';
import {Circle} from 'react-native-svg';
import type {ValueOf} from 'type-fest';
import useStyleUtils from '@hooks/useStyleUtils';
import useTheme from '@hooks/useTheme';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import SkeletonViewContentLoader from './SkeletonViewContentLoader';

function AvatarSkeleton() {
function AvatarSkeleton({size = CONST.AVATAR_SIZE.SMALL}: {size?: ValueOf<typeof CONST.AVATAR_SIZE>}) {
const theme = useTheme();
const skeletonCircleRadius = variables.sidebarAvatarSize / 2;

const StyleUtils = useStyleUtils();
const avatarSize = StyleUtils.getAvatarSize(size);
const skeletonCircleRadius = avatarSize / 2;

return (
<SkeletonViewContentLoader
animate
height={variables.sidebarAvatarSize}
height={avatarSize}
backgroundColor={theme.skeletonLHNIn}
foregroundColor={theme.skeletonLHNOut}
>
Expand Down
8 changes: 6 additions & 2 deletions src/components/AvatarWithImagePicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ function AvatarWithImagePicker({
setError(null, {});
}, [isFocused]);

useEffect(() => {
setError(null, {});
}, [source, avatarID]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed fix #48252

fix3.mov


/**
* Check if the attachment extension is allowed.
*/
Expand Down Expand Up @@ -325,7 +329,7 @@ function AvatarWithImagePicker({
);

return (
<View style={style}>
<View style={[styles.w100, style]}>
<View style={styles.w100}>
<AttachmentModal
headerTitle={headerTitle}
Expand Down Expand Up @@ -372,7 +376,7 @@ function AvatarWithImagePicker({
accessibilityLabel={translate('avatarWithImagePicker.editImage')}
disabled={isAvatarCropModalOpen || (disabled && !enablePreview)}
disabledStyle={disabledStyle}
style={[styles.pRelative, avatarStyle, type === CONST.ICON_TYPE_AVATAR && styles.alignSelfCenter]}
style={[styles.pRelative, type === CONST.ICON_TYPE_AVATAR && styles.alignSelfCenter, avatarStyle]}
ref={anchorRef}
>
<OfflineWithFeedback pendingAction={pendingAction}>
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/Delegate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Onyx.connect({
},
});

const KEYS_TO_PRESERVE_DELEGATE_ACCESS = [ONYXKEYS.NVP_TRY_FOCUS_MODE, ONYXKEYS.PREFERRED_THEME, ONYXKEYS.NVP_PREFERRED_LOCALE, ONYXKEYS.SESSION];
const KEYS_TO_PRESERVE_DELEGATE_ACCESS = [ONYXKEYS.NVP_TRY_FOCUS_MODE, ONYXKEYS.PREFERRED_THEME, ONYXKEYS.NVP_PREFERRED_LOCALE, ONYXKEYS.SESSION, ONYXKEYS.IS_LOADING_APP];

function connect(email: string) {
if (!delegatedAccess?.delegators) {
Expand Down
48 changes: 27 additions & 21 deletions src/pages/settings/Profile/ProfilePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import AvatarSkeleton from '@components/AvatarSkeleton';
import AvatarWithImagePicker from '@components/AvatarWithImagePicker';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
Expand Down Expand Up @@ -34,6 +35,7 @@ import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {LoginList, PrivatePersonalDetails} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

type ProfilePageOnyxProps = {
loginList: OnyxEntry<LoginList>;
Expand Down Expand Up @@ -155,27 +157,31 @@ function ProfilePage({
titleStyles={styles.accountSettingsSectionTitle}
>
<View style={[styles.pt3, styles.pb6, styles.alignSelfStart]}>
<MenuItemGroup shouldUseSingleExecution={false}>
<AvatarWithImagePicker
isUsingDefaultAvatar={UserUtils.isDefaultAvatar(currentUserPersonalDetails?.avatar ?? '')}
source={avatarURL}
avatarID={accountID}
onImageSelected={PersonalDetails.updateAvatar}
onImageRemoved={PersonalDetails.deleteAvatar}
size={CONST.AVATAR_SIZE.XLARGE}
avatarStyle={styles.avatarXLarge}
pendingAction={currentUserPersonalDetails?.pendingFields?.avatar ?? undefined}
errors={currentUserPersonalDetails?.errorFields?.avatar ?? null}
errorRowStyles={styles.mt6}
onErrorClose={PersonalDetails.clearAvatarErrors}
onViewPhotoPress={() => Navigation.navigate(ROUTES.PROFILE_AVATAR.getRoute(String(accountID)))}
previewSource={UserUtils.getFullSizeAvatar(avatarURL, accountID)}
originalFileName={currentUserPersonalDetails.originalFileName}
headerTitle={translate('profilePage.profileAvatar')}
fallbackIcon={currentUserPersonalDetails?.fallbackIcon}
editIconStyle={styles.profilePageAvatar}
/>
</MenuItemGroup>
{isEmptyObject(currentUserPersonalDetails) || accountID === -1 || !avatarURL ? (
<AvatarSkeleton size={CONST.AVATAR_SIZE.XLARGE} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmed fix #48259

) : (
<MenuItemGroup shouldUseSingleExecution={false}>
<AvatarWithImagePicker
isUsingDefaultAvatar={UserUtils.isDefaultAvatar(currentUserPersonalDetails?.avatar ?? '')}
source={avatarURL}
avatarID={accountID}
onImageSelected={PersonalDetails.updateAvatar}
onImageRemoved={PersonalDetails.deleteAvatar}
size={CONST.AVATAR_SIZE.XLARGE}
avatarStyle={[styles.avatarXLarge, styles.alignSelfStart]}
pendingAction={currentUserPersonalDetails?.pendingFields?.avatar ?? undefined}
errors={currentUserPersonalDetails?.errorFields?.avatar ?? null}
errorRowStyles={styles.mt6}
onErrorClose={PersonalDetails.clearAvatarErrors}
onViewPhotoPress={() => Navigation.navigate(ROUTES.PROFILE_AVATAR.getRoute(String(accountID)))}
previewSource={UserUtils.getFullSizeAvatar(avatarURL, accountID)}
originalFileName={currentUserPersonalDetails.originalFileName}
headerTitle={translate('profilePage.profileAvatar')}
fallbackIcon={currentUserPersonalDetails?.fallbackIcon}
editIconStyle={styles.profilePageAvatar}
/>
</MenuItemGroup>
)}
</View>
{publicOptions.map((detail, index) => (
<MenuItemWithTopDescription
Expand Down
4 changes: 2 additions & 2 deletions src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function useOptions() {
0,
);

const headerMessage = OptionsListUtils.getHeaderMessage((recentReports?.length || 0) + (personalDetails?.length || 0) !== 0 || !!currentUserOption, !!userToInvite, '');
const headerMessage = OptionsListUtils.getHeaderMessage((recentReports?.length || 0) + (personalDetails?.length || 0) !== 0, !!userToInvite, '');

if (isLoading) {
setIsLoading(false);
Expand All @@ -72,7 +72,7 @@ function useOptions() {
maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW,
});
const headerMessage = OptionsListUtils.getHeaderMessage(
(filteredOptions.recentReports?.length || 0) + (filteredOptions.personalDetails?.length || 0) !== 0 || !!filteredOptions.currentUserOption,
(filteredOptions.recentReports?.length || 0) + (filteredOptions.personalDetails?.length || 0) !== 0,
!!filteredOptions.userToInvite,
debouncedSearchValue,
);
Expand Down
3 changes: 2 additions & 1 deletion src/pages/settings/Security/AddDelegate/SelectDelegateRolePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ function SelectDelegateRolePage({route}: SelectDelegateRolePageProps) {
const roleOptions = Object.values(CONST.DELEGATE_ROLE).map((role) => ({
value: role,
text: translate('delegate.role', role),
keyForList: role,
alternateText: translate('delegate.roleDescription', role),
isSelected: role === route.params.role,
keyForList: role,
}));

return (
Expand All @@ -41,6 +41,7 @@ function SelectDelegateRolePage({route}: SelectDelegateRolePageProps) {
<SelectionList
isAlternateTextMultilineSupported
alternateTextNumberOfLines={4}
initiallyFocusedOptionKey={route.params.role}
rushatgabhane marked this conversation as resolved.
Show resolved Hide resolved
rushatgabhane marked this conversation as resolved.
Show resolved Hide resolved
headerContent={
<Text style={[styles.ph5, styles.pb5, styles.pt3]}>
<>
Expand Down
6 changes: 3 additions & 3 deletions src/pages/settings/Security/SecuritySettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ function SecuritySettingsPage() {
description: personalDetail?.displayName ? formattedEmail : '',
badgeText: translate('delegate.role', role),
avatarID: personalDetail?.accountID ?? -1,
icon: personalDetail?.avatar ?? '',
icon: personalDetail?.avatar ?? FallbackAvatar,
iconType: CONST.ICON_TYPE_AVATAR,
numberOfLinesDescription: 1,
wrapperStyle: [styles.sectionMenuItemTopDescription],
Expand Down Expand Up @@ -163,15 +163,15 @@ function SecuritySettingsPage() {
<Section
title={translate('delegate.copilotDelegatedAccess')}
renderSubtitle={() => (
<View style={[styles.flexRow, styles.alignItemsCenter, styles.w100, styles.mt2]}>
<Text style={[styles.flexRow, styles.alignItemsCenter, styles.w100, styles.mt2]}>
<Text style={[styles.textNormal, styles.colorMuted]}>{translate('delegate.copilotDelegatedAccessDescription')} </Text>
<TextLink
style={[styles.link]}
href={CONST.COPILOT_HELP_URL}
>
{translate('common.learnMore')}
</TextLink>
</View>
</Text>
)}
isCentralPane
subtitleMuted
Expand Down
7 changes: 7 additions & 0 deletions src/pages/settings/Subscription/SubscriptionSettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React, {useEffect} from 'react';
import {View} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
Expand All @@ -11,6 +13,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import NotFoundPage from '@pages/ErrorPage/NotFoundPage';
import * as Subscription from '@userActions/Subscription';
import ONYXKEYS from '@src/ONYXKEYS';
import CardSection from './CardSection/CardSection';
import ReducedFunctionalityMessage from './ReducedFunctionalityMessage';
import SubscriptionDetails from './SubscriptionDetails';
Expand All @@ -26,7 +29,11 @@ function SubscriptionSettingsPage() {
useEffect(() => {
Subscription.openSubscriptionPage();
}, []);
const [isAppLoading] = useOnyx(ONYXKEYS.IS_LOADING_APP);

if (!subscriptionPlan && isAppLoading) {
return <FullScreenLoadingIndicator />;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed fix #48246

fix2.mov

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rushatgabhane I still see not here page for a brief amount of time, is that expected?

Screen.Recording.2024-09-16.at.9.19.42.PM.mov

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is worth improving this more. The subscription data is simply not in Onyx, so we have to show not found page

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the conditon now. the most we can do is to show a loader when the app is loading.

    if (!subscriptionPlan && isAppLoading) {
        return <FullScreenLoadingIndicator />;
    }
    if (!subscriptionPlan) {
        return <NotFoundPage />;
    }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that's fine for now

if (!subscriptionPlan) {
return <NotFoundPage />;
}
Expand Down
Loading