Implement trimming of leading/trailing whitespace and newlines for verification codes #586
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/router/src/guardian-ui/components/setup/screens/verifyGuardians/VerifyGuardians.tsxOops! Something went wrong! :( ESLint: 8.40.0 Error: Cannot read config file: /apps/router/.eslintrc.js
📝 WalkthroughWalkthroughThe pull request introduces a new custom hook Changes
Sequence DiagramsequenceDiagram
participant Component as VerifyGuardians
participant Hook as useTrimmedInputArray
participant State as Input State
Component->>Hook: Initialize with empty values
Hook-->>Component: Return values and handleChange
Component->>Hook: Call handleChange with index and value
Hook->>State: Update trimmed value at specific index
State-->>Component: Reflect updated input state
The sequence diagram illustrates how the new Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/router/src/hooks/custom/useTrimmedInput.tsx (1)
3-3: Memoize cleanInput functionThe
cleanInputfunction should be memoized usinguseCallbackto prevent unnecessary re-renders.-const cleanInput = (value: string) => value.trim(); +const cleanInput = useCallback((value: string) => value.trim(), []);apps/router/src/guardian-ui/components/setup/screens/verifyGuardians/VerifyGuardians.tsx (1)
Line range hint
138-141: Memoize isAllValid checkThe
isAllValidcheck should be memoized to prevent unnecessary recalculations.-const isAllValid = - peersWithHash && - peersWithHash.every(({ hash }, idx) => hash === enteredHashes[idx]); +const isAllValid = useMemo( + () => + peersWithHash && + peersWithHash.every(({ hash }, idx) => hash === enteredHashes[idx]), + [peersWithHash, enteredHashes] +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/router/src/guardian-ui/components/setup/screens/verifyGuardians/VerifyGuardians.tsx(5 hunks)apps/router/src/hooks/custom/useTrimmedInput.tsx(1 hunks)apps/router/src/hooks/index.tsx(1 hunks)
🔇 Additional comments (1)
apps/router/src/hooks/index.tsx (1)
6-6: LGTM! Verify the exported module exists.
The new export follows the established pattern and supports the PR's objective.
✅ Verification successful
The exported module exists at the expected location
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that the exported module exists
fd --type f "useTrimmedInput.tsx" apps/router/src/hooks/custom
Length of output: 111
| const handleChange = (index: number, newValue: string) => { | ||
| setValues((prev) => { | ||
| const newValues = [...prev]; | ||
| newValues[index] = cleanInput(newValue); | ||
| return newValues; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Add index bounds validation
The handleChange function should validate the index to prevent array out-of-bounds errors.
const handleChange = (index: number, newValue: string) => {
+ if (index < 0 || index >= values.length) {
+ throw new Error('Index out of bounds');
+ }
setValues((prev) => {
const newValues = [...prev];
newValues[index] = cleanInput(newValue);
return newValues;
});
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleChange = (index: number, newValue: string) => { | |
| setValues((prev) => { | |
| const newValues = [...prev]; | |
| newValues[index] = cleanInput(newValue); | |
| return newValues; | |
| }); | |
| }; | |
| const handleChange = (index: number, newValue: string) => { | |
| if (index < 0 || index >= values.length) { | |
| throw new Error('Index out of bounds'); | |
| } | |
| setValues((prev) => { | |
| const newValues = [...prev]; | |
| newValues[index] = cleanInput(newValue); | |
| return newValues; | |
| }); | |
| }; |
| // Prefill hashes if already verified | ||
| if ( | ||
| peers[ourCurrentId].status === GuardianServerStatus.VerifiedConfigs | ||
| ) { | ||
| const otherPeers = Object.entries(peers).filter( | ||
| ([id]) => id !== ourCurrentId.toString() | ||
| ); | ||
| setEnteredHashes( | ||
| otherPeers.map(([id]) => hashes[id as unknown as number]) | ||
| ); | ||
| otherPeers.forEach(([id], idx) => { | ||
| handleHashChange(idx, hashes[id as unknown as number]); | ||
| }); |
There was a problem hiding this comment.
Clear error state before retrying
The error state should be cleared before attempting to prefill hashes to prevent stale error messages.
// Prefill hashes if already verified
+setError(undefined);
if (peers[ourCurrentId].status === GuardianServerStatus.VerifiedConfigs) {
const otherPeers = Object.entries(peers).filter(
([id]) => id !== ourCurrentId.toString()
);
otherPeers.forEach(([id], idx) => {
handleHashChange(idx, hashes[id as unknown as number]);
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Prefill hashes if already verified | |
| if ( | |
| peers[ourCurrentId].status === GuardianServerStatus.VerifiedConfigs | |
| ) { | |
| const otherPeers = Object.entries(peers).filter( | |
| ([id]) => id !== ourCurrentId.toString() | |
| ); | |
| setEnteredHashes( | |
| otherPeers.map(([id]) => hashes[id as unknown as number]) | |
| ); | |
| otherPeers.forEach(([id], idx) => { | |
| handleHashChange(idx, hashes[id as unknown as number]); | |
| }); | |
| // Prefill hashes if already verified | |
| setError(undefined); | |
| if ( | |
| peers[ourCurrentId].status === GuardianServerStatus.VerifiedConfigs | |
| ) { | |
| const otherPeers = Object.entries(peers).filter( | |
| ([id]) => id !== ourCurrentId.toString() | |
| ); | |
| otherPeers.forEach(([id], idx) => { | |
| handleHashChange(idx, hashes[id as unknown as number]); | |
| }); |
| } | ||
| } catch (err) { | ||
| setError(formatApiErrorMessage(err)); | ||
| } | ||
| } | ||
| assembleHashInfo(); | ||
| }, [api, peers, ourCurrentId, t]); | ||
| }, [api, peers, ourCurrentId, t, handleHashChange]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove handleHashChange from useEffect dependencies
Including handleHashChange in the dependency array could cause unnecessary re-renders. Consider wrapping the hash prefill logic in a separate useCallback.
-}, [api, peers, ourCurrentId, t, handleHashChange]);
+const prefillHashes = useCallback((hashes) => {
+ const otherPeers = Object.entries(peers).filter(
+ ([id]) => id !== ourCurrentId.toString()
+ );
+ otherPeers.forEach(([id], idx) => {
+ handleHashChange(idx, hashes[id as unknown as number]);
+ });
+}, [peers, ourCurrentId, handleHashChange]);
+
+}, [api, peers, ourCurrentId, t, prefillHashes]);Committable suggestion skipped: line range outside the PR's diff.
kleysc
changed the title
Issue #585
Implementation Summary
Objective: Implement a hook to trim leading and trailing whitespace from verification codes, improving user input validation and enhancing the overall experience during setup.
Key Changes
Custom Hook
useTrimmedInputArray:ui/apps/router/src/hooks/custom/useTrimmedInput.tsx.Updated Components:
VerifyGuardians.tsx: IntegrateduseTrimmedInputArrayto sanitize the verification codes entered by guardians.Features
Before and Now
useTrimmedInputArrayensures all inputs are sanitized automatically, improving robustness and maintainability.Summary by CodeRabbit
New Features
VerifyGuardianscomponent for improved input handling and verification of guardian hashes.Bug Fixes
Documentation