Skip to content

Commit f218f12

Browse files
authored
Merge pull request #3491 from ecency/bugfix/pre-release-hardening
Pre-release hardening: parseAsset, sheet model, navigation contract, delete default
2 parents 6a3f8b8 + fc224f7 commit f218f12

25 files changed

Lines changed: 283 additions & 117 deletions

File tree

src/components/accountsBottomSheet/container/accountsBottomSheetContainer.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import { useIntl } from 'react-intl';
55
import { Alert } from 'react-native';
66
import { SheetManager } from 'react-native-actions-sheet';
77
import { getMutedUsersQueryOptions, getNotificationsUnreadCountQueryOptions } from '@ecency/sdk';
8-
import RootNavigation from '../../../navigation/rootNavigation';
8+
import RootNavigation, { NavigateOptions } from '../../../navigation/rootNavigation';
9+
import { NavigateArgs, RouteName } from '../../../navigation/types';
910

1011
import { setPrevLoggedInUsers, updateCurrentAccount } from '../../../redux/actions/accountAction';
1112

@@ -66,11 +67,12 @@ const AccountsBottomSheetContainer = () => {
6667
}
6768
};
6869

69-
const _navigateToRoute = (name: string, params: any) => {
70+
const _navigateToRoute = <K extends RouteName>(...[name, params]: NavigateArgs<K>) => {
7071
SheetManager.hide(SheetNames.ACCOUNTS_SHEET);
7172
accountsBottomSheetViewRef.current?.closeAccountsBottomSheet();
7273
if (name) {
73-
RootNavigation.navigate({ name, params });
74+
// Correlated by the generic at the call site; TS cannot re-derive that pairing here.
75+
RootNavigation.navigate({ name, params } as NavigateOptions);
7476
}
7577
};
7678

src/components/accountsBottomSheet/view/accountsBottomSheetView.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import HiveAuthIconSource from '../../../assets/HiveAuth_logo.png';
1515
import HiveIconSource from '../../../assets/hive_icon.png';
1616

1717
import { default as ROUTES } from '../../../constants/routeNames';
18+
import { NavigateArgs, RouteName } from '../../../navigation/types';
1819

1920
import styles from './accountsBottomSheetStyles';
2021

@@ -26,7 +27,7 @@ export interface AccountsBottomSheetRef {
2627
export interface AccountsBottomSheetProps {
2728
accounts: any[];
2829
currentAccount: any;
29-
navigateToRoute: (route: string, params?: any) => void;
30+
navigateToRoute: <K extends RouteName>(...args: NavigateArgs<K>) => void;
3031
switchAccount: (account: any) => void;
3132
prevLoggedInUsers: Array<any>;
3233
dispatch: (action: any) => void;

src/components/aiAssistModal/aiAssistModal.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ export const AiAssistModal = ({ payload }: SheetProps<SheetNames.AI_ASSIST>) =>
5252
const [result, setResult] = useState<{ output: string; action: AiAssistAction } | null>(null);
5353
const [selectedTitleIndex, setSelectedTitleIndex] = useState(0);
5454

55-
// Reset state when payload changes (sheets stay mounted)
55+
// Sheets mount on show, so this seeds the input on every open. The `payload` dep additionally
56+
// covers a re-show onto a still-mounted sheet.
5657
useEffect(() => {
5758
const newText = payload?.text?.slice(0, MAX_INPUT) || '';
5859
setText(newText);

src/components/balanceAnalyticsSheet/balanceAnalyticsSheet.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,9 @@ const BalanceAnalyticsSheet = ({ payload }: SheetProps<SheetNames.BALANCE_ANALYT
8383
const [activeTab, setActiveTab] = useState<Tab>('history');
8484
const [granularity, setGranularity] = useState<Granularity>('yearly');
8585

86-
// Action sheets stay mounted between presentations (per CLAUDE.md
87-
// sheet-pattern note), so reset our local view state whenever the sheet is
88-
// re-opened with a new payload — otherwise the previous account/coin's tab
89-
// and granularity selection leak into the new view.
86+
// Sheets mount on show, so useState already seeds these on a normal open. This effect covers
87+
// a re-show onto a still-mounted sheet with a different account/coin, whose tab and
88+
// granularity selection would otherwise leak into the new view.
9089
useEffect(() => {
9190
setActiveTab('history');
9291
setGranularity('yearly');

src/components/communityRoleEditSheet/communityRoleEditSheet.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ const CommunityRoleEditSheet: React.FC<SheetProps<'community_role_edit'>> = ({
9494
setError('');
9595
}, [payload?.account]);
9696

97-
// Registered sheets stay mounted, and resetting on payload identity alone is
98-
// not enough when the same payload object is reused, so reset on every
99-
// presentation.
97+
// Sheets mount on show, so mounting is what guarantees the reset on every presentation. The
98+
// `payload` dep only covers a re-show onto a still-mounted sheet, and it cannot be relied on
99+
// alone: a caller reusing the same payload object would not re-fire it.
100100
useEffect(() => {
101101
_reset();
102102
}, [payload, _reset]);

src/components/composeTranslateModal/composeTranslateModal.tsx

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,16 @@ export const ComposeTranslateModal = ({ payload }: SheetProps<SheetNames.COMPOSE
5656
const [progress, setProgress] = useState<[number, number]>([0, 0]);
5757
const [failed, setFailed] = useState(false);
5858

59-
// Sheets stay mounted; a closed sheet cancels the request chain so it stops
60-
// hitting the translation service in the background.
61-
const closedRef = useRef(false);
59+
// Identifies the current translation run. A run is superseded by closing the sheet, by
60+
// unmount, by a re-show carrying a new payload, by changing the language pair, and by
61+
// starting another translation. translateMarkdown pages the service request by request and
62+
// only consults its cancel callback between pages, so a run always has to be assumed still
63+
// in flight: everything it writes back is gated on still being the current run.
64+
//
65+
// A plain "closed" flag was not enough. It was cleared again on re-show, and the result
66+
// handler did not consult it at all, so a run that resolved after the user switched target
67+
// language applied its result to the new pair, defeating the reset below.
68+
const runIdRef = useRef(0);
6269

6370
const sample = useMemo(
6471
() =>
@@ -72,10 +79,19 @@ export const ComposeTranslateModal = ({ payload }: SheetProps<SheetNames.COMPOSE
7279
);
7380
const tooShort = sample.trim().length < MIN_TRANSLATE_CHARS;
7481

75-
// Reset state and re-detect the source language on every open (payload is a
76-
// fresh object per SheetManager.show).
82+
// Sheets unmount on hide, so the run has to be retired here as well: onClose is not
83+
// guaranteed to be the route the sheet leaves by, and a run left current keeps paging the
84+
// translation service for a screen the user has already dismissed.
85+
useEffect(
86+
() => () => {
87+
runIdRef.current += 1;
88+
},
89+
[],
90+
);
91+
92+
// Sheets mount on show, so this resets state and re-detects the source language on every open.
7793
useEffect(() => {
78-
closedRef.current = false;
94+
runIdRef.current += 1;
7995
setTranslated('');
8096
setFailed(false);
8197
setTranslating(false);
@@ -111,8 +127,16 @@ export const ComposeTranslateModal = ({ payload }: SheetProps<SheetNames.COMPOSE
111127
setTarget(reader && reader !== 'en' && LIBRETRANSLATE_TARGETS.has(reader) ? reader : 'es');
112128
}, [source, appLang]);
113129

114-
// A result translated into a previous language pair must never be applied.
130+
// A result translated into a previous language pair must never be applied. Retiring the run
131+
// is what enforces that: clearing the state alone loses the race against a run still in
132+
// flight, which would write its result back a moment later.
133+
//
134+
// Clearing `translating` is part of retiring it. The run being cancelled can no longer do it
135+
// itself, because every write it makes is now gated on still being current, and a stuck
136+
// `translating` leaves the spinner up with both action buttons disabled.
115137
useEffect(() => {
138+
runIdRef.current += 1;
139+
setTranslating(false);
116140
setTranslated('');
117141
setFailed(false);
118142
}, [source, target]);
@@ -131,6 +155,10 @@ export const ComposeTranslateModal = ({ payload }: SheetProps<SheetNames.COMPOSE
131155
);
132156

133157
const _translate = async () => {
158+
runIdRef.current += 1;
159+
const runId = runIdRef.current;
160+
const isCurrent = () => runIdRef.current === runId;
161+
134162
setTranslating(true);
135163
setFailed(false);
136164
setTranslated('');
@@ -140,17 +168,23 @@ export const ComposeTranslateModal = ({ payload }: SheetProps<SheetNames.COMPOSE
140168
body,
141169
source,
142170
target,
143-
(done, total) => setProgress([done, total]),
144-
() => closedRef.current,
171+
(done, total) => {
172+
if (isCurrent()) {
173+
setProgress([done, total]);
174+
}
175+
},
176+
() => !isCurrent(),
145177
);
146-
setTranslated(result);
178+
if (isCurrent()) {
179+
setTranslated(result);
180+
}
147181
} catch (error) {
148182
console.log('translate error : ', error);
149-
if (!closedRef.current) {
183+
if (isCurrent()) {
150184
setFailed(true);
151185
}
152186
} finally {
153-
if (!closedRef.current) {
187+
if (isCurrent()) {
154188
setTranslating(false);
155189
}
156190
}
@@ -173,7 +207,7 @@ export const ComposeTranslateModal = ({ payload }: SheetProps<SheetNames.COMPOSE
173207
};
174208

175209
const _handleOnSheetClose = () => {
176-
closedRef.current = true;
210+
runIdRef.current += 1;
177211
setTranslating(false);
178212
};
179213

src/components/foregroundNotification/foregroundNotification.tsx

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -128,31 +128,23 @@ const ForegroundNotification = ({ remoteMessage }: Props) => {
128128
const { data } = remoteMessage;
129129
const { type } = data;
130130

131-
let routeName;
132-
let params;
133-
let key;
134-
135131
if (type === 'transfer' || type === 'delegations') {
136132
// Navigate to wallet for financial transactions
137-
routeName = ROUTES.TABBAR.WALLET;
133+
RootNavigation.navigate({ name: ROUTES.TABBAR.WALLET });
138134
} else {
139135
// Navigate to post for reply/mention
140136
const fullPermlink =
141137
get(data, 'permlink1', '') + get(data, 'permlink2', '') + get(data, 'permlink3', '');
142138

143-
params = {
144-
author: get(data, 'source', ''),
145-
permlink: fullPermlink,
146-
};
147-
key = fullPermlink;
148-
routeName = ROUTES.SCREENS.POST;
139+
RootNavigation.navigate({
140+
name: ROUTES.SCREENS.POST,
141+
params: {
142+
author: get(data, 'source', ''),
143+
permlink: fullPermlink,
144+
},
145+
key: fullPermlink,
146+
});
149147
}
150-
151-
RootNavigation.navigate({
152-
name: routeName,
153-
params,
154-
key,
155-
});
156148
hide();
157149
};
158150

src/components/iconButton/view/iconButtonView.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import React, { Fragment } from 'react';
2-
import { TouchableOpacity, ActivityIndicator, StyleProp, TextStyle, ViewStyle } from 'react-native';
2+
import {
3+
TouchableOpacity,
4+
TouchableOpacityProps,
5+
ActivityIndicator,
6+
StyleProp,
7+
TextStyle,
8+
ViewStyle,
9+
} from 'react-native';
310
import EStyleSheet from 'react-native-extended-stylesheet';
411
import { Icon } from '../../icon';
512

@@ -23,6 +30,9 @@ interface IconButtonProps {
2330
isLoading?: boolean;
2431
iconStyle?: StyleProp<TextStyle>;
2532
style?: StyleProp<ViewStyle>;
33+
// The button is a fixed 30x30, below the 44pt guideline. Opt in per call site where a
34+
// mis-tap is costly, rather than changing the hit area of every icon button at once.
35+
hitSlop?: TouchableOpacityProps['hitSlop'];
2636
onPress?: (event?: any) => void;
2737
onLongPress?: (event?: any) => void;
2838
accessibilityLabel?: string;
@@ -46,10 +56,12 @@ const IconButton = ({
4656
isLoading,
4757
accessibilityLabel,
4858
accessibilityHint,
59+
hitSlop,
4960
}: IconButtonProps) => (
5061
<Fragment>
5162
<TouchableOpacity
5263
style={[styles.iconButton, style]}
64+
hitSlop={hitSlop}
5365
onPress={() => !isLoading && onPress && onPress()}
5466
disabled={disabled}
5567
onLongPress={() => !isLoading && onLongPress && onLongPress()}

src/components/post-translation-modal/postTranslationModal.tsx

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,9 @@ const PostTranslationModal = ({ payload }: SheetProps<'post_translation'>) => {
4747
getSupportedLanguages();
4848
}, []);
4949

50-
// Sheets stay mounted and _handleOnSheetClose resets the target on close, so
51-
// re-apply the pre-selected target/source on EVERY open. Keying on `payload`
52-
// (a fresh object per SheetManager.show) is what makes this re-fire when the
53-
// sheet is reopened from the same chip/banner with unchanged initial codes —
54-
// without it the pre-targeting would only work once per session.
50+
// Sheets mount on show, so this applies the pre-selected target/source on every open. It has
51+
// to run again once the language list resolves, which is why the list is a dependency: on the
52+
// first open the codes arrive before the list and there is nothing to match them against yet.
5553
useEffect(() => {
5654
if (!supportedLangsList.length) {
5755
return;

src/components/postElements/body/view/commentBodyView.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,11 @@ const CommentBody = ({
7373
return;
7474
}
7575
const name = isCommunity(tag) ? ROUTES.SCREENS.COMMUNITY : ROUTES.SCREENS.TAG_RESULT;
76-
const key = `${filter}/${tag}`;
7776
RootNavigation.navigate({
7877
name,
7978
params: {
8079
tag,
8180
filter,
82-
key,
8381
},
8482
});
8583
}

0 commit comments

Comments
 (0)