feat(mobile): replace SAP landing screen with Android notes app#1
feat(mobile): replace SAP landing screen with Android notes app#1
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the complex SAP WMS mobile application with a simplified notes application that uses AsyncStorage for local persistence. The review feedback identifies a critical race condition in the persistence logic where the initial empty state could overwrite existing data before the loading process completes. Additionally, there is a recommendation to optimize date formatting within the list's render function to prevent performance issues and ensure consistent behavior across different mobile JavaScript engines.
| const [notes, setNotes] = useState<Note[]>([]); | ||
| const [inputValue, setInputValue] = useState(""); | ||
|
|
||
| useEffect(() => { | ||
| if (!page) return; | ||
|
|
||
| if (hasBlockingError(page)) { | ||
| if (page.kind === "login") { | ||
| setStage("login"); | ||
| } else if (page.kind === "rfui-logon" || page.kind === "rfui-error") { | ||
| setStage("warehouse"); | ||
| } | ||
| if (page.messages.length) { | ||
| setErrorText(page.messages[0]); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| if (stage === "boot") { | ||
| if (page.kind === "login") { | ||
| setStage("login"); | ||
| setInfoText("Sign in with your SAP account."); | ||
| } else if (page.kind === "rfui-logon") { | ||
| setStage("warehouse"); | ||
| setInfoText("SAP session found. Complete warehouse setup."); | ||
| } else if (isRealConnectedRfui(page)) { | ||
| setStage("ready"); | ||
| setInfoText("Connected to SAP."); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| if (stage === "authenticating") { | ||
| if (page.kind === "rfui-logon") { | ||
| setStage("warehouse"); | ||
| setErrorText(""); | ||
| setInfoText("SAP login successful. Complete warehouse setup."); | ||
| } else if (isRealConnectedRfui(page)) { | ||
| setStage("ready"); | ||
| setErrorText(""); | ||
| setInfoText("SAP login successful."); | ||
| } else if (page.kind === "login" && page.messages.length) { | ||
| setStage("login"); | ||
| setErrorText(page.messages[0]); | ||
| void (async () => { | ||
| const raw = await AsyncStorage.getItem(STORAGE_KEY); | ||
| if (!raw) return; | ||
| try { | ||
| const parsed = JSON.parse(raw) as Note[]; | ||
| setNotes(parsed); | ||
| } catch { | ||
| await AsyncStorage.removeItem(STORAGE_KEY); | ||
| } | ||
| return; | ||
| } | ||
| })(); | ||
| }, []); | ||
|
|
||
| if (stage === "submittingWarehouse") { | ||
| if (isRealConnectedRfui(page)) { | ||
| setStage("ready"); | ||
| setErrorText(""); | ||
| setInfoText("Warehouse session connected successfully."); | ||
| } else if ( | ||
| page.kind === "rfui-logon" || | ||
| page.kind === "rfui-error" || | ||
| page.messages.length | ||
| ) { | ||
| setStage("warehouse"); | ||
| if (page.messages.length) { | ||
| setErrorText(page.messages[0]); | ||
| } | ||
| } | ||
| } | ||
| }, [page, stage]); | ||
|
|
||
| const inject = (script: string) => { | ||
| webViewRef.current?.injectJavaScript(script); | ||
| }; | ||
|
|
||
| const injectInspector = () => { | ||
| inject(INSPECT_PAGE_JS); | ||
| }; | ||
|
|
||
| const navigateHome = () => { | ||
| setErrorText(""); | ||
| setInfoText("Opening SAP..."); | ||
| inject(`window.location.href = ${escapeForInjectedJs(SAP_RFUI_URL)}; true;`); | ||
| }; | ||
|
|
||
| const handleNavigationChange = (navState: WebViewNavigation) => { | ||
| setCanGoBack(navState.canGoBack); | ||
| }; | ||
|
|
||
| const handleMessage = (event: WebViewMessageEvent) => { | ||
| try { | ||
| const data = JSON.parse(event.nativeEvent.data); | ||
|
|
||
| if (data.type === "PAGE_STATE" && data.payload) { | ||
| const nextPage: SapPageSnapshot = { | ||
| url: data.payload.url ?? "", | ||
| host: data.payload.host ?? "", | ||
| title: data.payload.title ?? "SAP", | ||
| kind: data.payload.kind ?? "unknown", | ||
| messages: Array.isArray(data.payload.messages) ? data.payload.messages : [], | ||
| hasWarehouse: !!data.payload.hasWarehouse, | ||
| hasResource: !!data.payload.hasResource, | ||
| hasDevice: !!data.payload.hasDevice, | ||
| warehouseValue: data.payload.warehouseValue ?? "", | ||
| resourceValue: data.payload.resourceValue ?? "", | ||
| deviceValue: data.payload.deviceValue ?? "", | ||
| isErrorPage: !!data.payload.isErrorPage, | ||
| }; | ||
|
|
||
| setPage(nextPage); | ||
|
|
||
| if (nextPage.messages.length && stage !== "ready") { | ||
| setErrorText(nextPage.messages[0]); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| if (data.type === "ACTION_RESULT") { | ||
| if (!data.ok && data.error) { | ||
| setErrorText(String(data.error)); | ||
| if (data.action === "login") { | ||
| setStage("login"); | ||
| } else if (data.action === "warehouse") { | ||
| setStage("warehouse"); | ||
| } | ||
| } | ||
| } | ||
| } catch {} | ||
| }; | ||
|
|
||
| const submitLogin = () => { | ||
| const trimmedUser = username.trim(); | ||
| const trimmedPass = password; | ||
|
|
||
| if (!trimmedUser || !trimmedPass) { | ||
| setErrorText("Enter your SAP email/username and password."); | ||
| return; | ||
| } | ||
|
|
||
| if (page?.kind === "rfui-logon") { | ||
| setStage("warehouse"); | ||
| setErrorText(""); | ||
| return; | ||
| } | ||
|
|
||
| if (isRealConnectedRfui(page)) { | ||
| setStage("ready"); | ||
| setErrorText(""); | ||
| return; | ||
| } | ||
|
|
||
| setErrorText(""); | ||
| setInfoText("Signing in to SAP..."); | ||
| setStage("authenticating"); | ||
| inject(createLoginInjection(trimmedUser, trimmedPass, keepSignedIn)); | ||
| }; | ||
| useEffect(() => { | ||
| void AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(notes)); | ||
| }, [notes]); |
There was a problem hiding this comment.
There is a race condition in the persistence logic. The useEffect responsible for saving notes (lines 40-42) runs on the initial render because notes is initialized to an empty array. Since AsyncStorage operations are asynchronous, this effect might overwrite existing stored notes with an empty array before the loading effect (lines 27-38) completes. I recommend adding an initialization flag to ensure that saving only occurs after the initial data has been loaded.
const [notes, setNotes] = useState<Note[]>([]);
const [inputValue, setInputValue] = useState("");
const [isInitialized, setIsInitialized] = useState(false);
useEffect(() => {
void (async () => {
try {
const raw = await AsyncStorage.getItem(STORAGE_KEY);
if (raw) {
const parsed = JSON.parse(raw) as Note[];
setNotes(parsed);
}
} catch {
await AsyncStorage.removeItem(STORAGE_KEY);
} finally {
setIsInitialized(true);
}
})();
}, []);
useEffect(() => {
if (isInitialized) {
void AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(notes));
}
}, [notes, isInitialized]);
| <Text style={styles.noteText}>{item.text}</Text> | ||
| <View style={styles.noteFooter}> | ||
| <Text style={styles.noteDate}> | ||
| {new Date(item.createdAt).toLocaleString()} |
There was a problem hiding this comment.
Calling new Date().toLocaleString() inside renderItem can lead to performance issues as the list grows and may produce inconsistent results across different JavaScript engines (e.g., JSC vs Hermes on Android). It is better to format the date once when the note is created or use a memoized component for the list items.
Motivation
Description
artifacts/mobile/app/index.tsxthat implements a header, multiline input, add button, note cards, delete action, and empty state.AsyncStorageunder the keynotes-app-v1to load notes on startup and save on changes, including defensive handling of invalid stored payloads.createdAttimestamps, a note counter label derived fromnotes.length, and a styledFlatListrendering.Testing
pnpm --filter @workspace/mobile typecheck, which exited with errors; the reported TypeScript syntax errors are located inartifacts/mobile/app/warehouse.tsxand are pre-existing and unrelated to the notes screen change.Codex Task
Need help on this PR? Tag
@codesmithwith what you need.