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 send sheet stuck on loading #6119

Merged
merged 9 commits into from
Sep 20, 2024
Merged

Fix send sheet stuck on loading #6119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a4b2f42
fix stuck loading phase
walmat Sep 19, 2024
3fa0b3e
Update src/screens/SendSheet.js
walmat Sep 19, 2024
8cee9c6
fix deleting contact resetting toAddress to undefined
walmat Sep 19, 2024
fe9505c
fix some shit with ens names
walmat Sep 19, 2024
3f36f90
rm logs
walmat Sep 19, 2024
1ac6cb7
fix failed to send
walmat Sep 19, 2024
21a4c76
fix lint
walmat Sep 19, 2024
7adfdc8
prefer contact name over wallet label
walmat Sep 19, 2024
4b943f7
force linter 2 re-rerun
brunobar79 Sep 19, 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/__swaps__/screens/Swap/components/SearchInputButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const SearchInputButton = ({
outputSearchRef,
AnimatedSwapStyles,
} = useSwapContext();

const { hasClipboardData } = useClipboard();

const btnText = useDerivedValue(() => {
Expand Down Expand Up @@ -74,7 +75,7 @@ export const SearchInputButton = ({
const isInputSearchFocused = inputProgress.value === NavigationSteps.SEARCH_FOCUSED;
const isOutputSearchFocused = outputProgress.value === NavigationSteps.SEARCH_FOCUSED;
const isOutputTokenListFocused = outputProgress.value === NavigationSteps.TOKEN_LIST_FOCUSED;

const clipboardDataAvailable = hasClipboardData || IS_ANDROID;

const isPasteDisabled = output && !internalSelectedOutputAsset.value && isOutputTokenListFocused && !clipboardDataAvailable;
Expand Down
2 changes: 1 addition & 1 deletion src/components/fields/CheckboxField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export default function CheckboxField({
</Box>
<Box flexShrink={1}>
<AccentColorProvider color={customColor || action}>
<Text color={isChecked ? 'accent' : 'secondary80 (Deprecated)'} size="16px / 22px (Deprecated)" weight="bold">
<Text color={isChecked ? 'accent' : 'secondary80 (Deprecated)'} size="15pt" weight="bold">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for L2s this was overflowing onto the next line for Android so that's why I made this change for context. Not a super good fix but it's a patch

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this is somehow affecting iOS. Maybe safer to wrap in a platform check and keep ios the same?

{label}
</Text>
</AccentColorProvider>
Expand Down
22 changes: 15 additions & 7 deletions src/components/send/SendHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,15 @@ export default function SendHeader({
const isPreExistingContact = (contact?.nickname?.length || 0) > 0;

const name =
removeFirstEmojiFromString(userWallet?.label || contact?.nickname || nickname) || userWallet?.ens || contact?.ens || recipient;
removeFirstEmojiFromString(contact?.nickname || userWallet?.label || nickname) || userWallet?.ens || contact?.ens || recipient;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you assigned a nickname to a contact it was never for wallets that had ENS names


const handleNavigateToContact = useCallback(() => {
let nickname = profilesEnabled ? (!isHexString(recipient) ? recipient : null) : recipient;
let color = '';
const nickname = !isHexString(name) ? name : '';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we only want nicknames prefilled for ENS related names.

if (!profilesEnabled) {
color = contact?.color;
if (color !== 0 && !color) {
const emoji = profileUtils.addressHashedEmoji(hexAddress);
color = profileUtils.addressHashedColorIndex(hexAddress) || 0;
nickname = isHexString(recipient) ? emoji : `${emoji} ${recipient}`;
}
}

Expand All @@ -142,7 +140,7 @@ export default function SendHeader({
onRefocusInput,
type: 'contact_profile',
});
}, [contact, hexAddress, navigate, onRefocusInput, profilesEnabled, recipient]);
}, [contact, hexAddress, name, navigate, onRefocusInput, profilesEnabled, recipient]);

const handleOpenContactActionSheet = useCallback(async () => {
return showActionSheetWithOptions(
Expand All @@ -162,7 +160,7 @@ export default function SendHeader({
address: hexAddress,
nickname: name,
onDelete: () => {
onChangeAddressInput(contact?.ens);
onChangeAddressInput(contact?.ens ? contact?.ens : contact?.address);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay so basically this was triggering a setToAddress of '' for non-ENS accounts, which threw us into a never ending loading state until you reselected the account you want to send to.

},
removeContact: removeContact,
});
Expand All @@ -175,7 +173,17 @@ export default function SendHeader({
}
}
);
}, [contact?.ens, handleNavigateToContact, hexAddress, onRefocusInput, removeContact, setClipboard, name, onChangeAddressInput]);
}, [
hexAddress,
name,
removeContact,
onChangeAddressInput,
contact?.ens,
contact?.address,
handleNavigateToContact,
onRefocusInput,
setClipboard,
]);

const onChange = useCallback(
text => {
Expand Down
2 changes: 1 addition & 1 deletion src/screens/SendConfirmationSheet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ export const SendConfirmationSheet = () => {
return existingAcct;
}, [toAddress, userAccounts, watchedAccounts]);

let avatarName = removeFirstEmojiFromString(existingAccount?.label || contact?.nickname);
let avatarName = removeFirstEmojiFromString(contact?.nickname || existingAccount?.label);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay similarly to the other instance of this, we were never showing contact nicknames for accounts with ens names since we always preferred ens names over contact nicknames.


if (!avatarName) {
if (isValidDomainFormat(to)) {
Expand Down
15 changes: 10 additions & 5 deletions src/screens/SendSheet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ export default function SendSheet(props) {
})();
}
},
[amountDetails.assetAmount, goBack, isENS, isHardwareWallet, navigate, onSubmit, recipient, selected?.name, selected?.network]
[amountDetails, goBack, isENS, isHardwareWallet, navigate, onSubmit, recipient, selected?.name, selected?.network]
);

const { buttonDisabled, buttonLabel } = useMemo(() => {
Expand Down Expand Up @@ -713,8 +713,8 @@ export default function SendSheet(props) {
const isValid = checkIsValidAddressOrDomainFormat(text);
if (!isValid) {
setIsValidAddress();
setToAddress();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should only reset the toAddress here if it's not valid.

}
setToAddress();
setCurrentInput(text);
setRecipient(text);
setNickname(text);
Expand Down Expand Up @@ -764,7 +764,6 @@ export default function SendSheet(props) {

if (
!!accountAddress &&
amountDetails.assetAmount !== '' &&
Object.entries(selected).length &&
assetChainId === currentChainId &&
currentProviderChainId === currentChainId &&
Expand Down Expand Up @@ -813,6 +812,12 @@ export default function SendSheet(props) {

const isEmptyWallet = !sortedAssets?.length && !sendableUniqueTokens?.length;

const filteredUserAccountsFromContacts = useMemo(() => {
return userAccounts.filter(
account => !filteredContacts.some(contact => contact.address.toLowerCase() === account.address.toLowerCase())
);
}, [userAccounts, filteredContacts]);

Comment on lines +815 to +820
Copy link
Contributor Author

@walmat walmat Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

fixes not showing the account in the user wallets list if it exists in the contacts list

return (
<Container testID="send-sheet">
<SheetContainer>
Expand All @@ -832,7 +837,7 @@ export default function SendSheet(props) {
recipientFieldRef={recipientFieldRef}
removeContact={onRemoveContact}
showAssetList={showAssetList}
userAccounts={userAccounts}
userAccounts={filteredUserAccountsFromContacts}
watchedAccounts={watchedAccounts}
/>
{showEmptyState && (
Expand All @@ -848,7 +853,7 @@ export default function SendSheet(props) {
setNickname(nickname);
}}
removeContact={onRemoveContact}
userAccounts={userAccounts}
userAccounts={filteredUserAccountsFromContacts}
watchedAccounts={watchedAccounts}
/>
)}
Expand Down
Loading