[TASK-6981] (main) fix: disable autocorrect on address input #533
Conversation
…s-input [TASK-6944] fix: disable autocorrect on address input
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe pull request introduces modifications to several components, including Changes
Possibly related PRs
Suggested reviewers
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: 2
🧹 Outside diff range and nitpick comments (1)
src/components/Global/ValidatedInput/index.tsx (1)
88-90: Consider adding aria-label for better accessibilityWhen disabling auto-correction features, it's important to ensure users are aware of this behavior, especially those using screen readers.
Add an aria-label to indicate that automatic corrections are disabled:
autoCorrect="off" autoCapitalize="off" autoComplete="off" + aria-label={`${label} input field with automatic corrections disabled`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Global/RecipientInput/index.tsx(1 hunks)src/components/Global/ValidatedInput/index.tsx(1 hunks)
🔇 Additional comments (1)
src/components/Global/ValidatedInput/index.tsx (1)
88-90: Verify the impact on mobile browsers
These attributes might behave differently across various mobile browsers and operating systems. Please verify the behavior works as expected.
| autoCorrect="off" | ||
| autoCapitalize="off" | ||
| autoComplete="off" |
There was a problem hiding this comment.
💡 Codebase verification
Similar input components need the same autocorrect treatment
Found two additional components that handle address/recipient input and should have the same autocorrect prevention:
src/components/Global/AddressInput/index.tsxsrc/components/Global/ValidatedInput/index.tsx
Both components already have spellCheck="false" but are missing the autoCorrect, autoCapitalize, and autoComplete attributes that were added to RecipientInput. These attributes should be added to maintain consistent behavior across all address input fields.
🔗 Analysis chain
LGTM! Good practice for address input fields.
The addition of autoCorrect="off", autoCapitalize="off", and autoComplete="off" is appropriate for address input fields where automatic corrections can interfere with accurate address entry. These attributes complement the existing spellCheck="false" setting.
Let's verify if similar address input fields exist that might need the same treatment:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find other potential address input components that might need similar attributes
# Search for input elements with address-related terms
rg -l 'address|location|recipient' --type tsx
# Search for other input components that might need these attributes
ast-grep --pattern 'className="input-text'
Length of output: 120
Script:
#!/bin/bash
# Let's try different approaches to find similar components
# Search for files containing address-related terms with proper extension
rg -l 'address|location|recipient' -g '*.tsx' -g '*.jsx'
# Search for input components with text type
rg -l 'type=["'\'']text["'\'']' -g '*.tsx' -g '*.jsx'
# Search for components with autocorrect/autocomplete attributes
rg -l 'autoCorrect|autoComplete|autoCapitalize' -g '*.tsx' -g '*.jsx'
# Look for other recipient-related components
rg -l 'recipient' -g '*.tsx' -g '*.jsx'
Length of output: 4841
Script:
#!/bin/bash
# Let's check the content of potentially related input components
# Check GeneralRecipientInput component
rg -A 5 'input' src/components/Global/GeneralRecipientInput/index.tsx
# Check AddressInput component
rg -A 5 'input' src/components/Global/AddressInput/index.tsx
# Check ValidatedInput component
rg -A 5 'input' src/components/Global/ValidatedInput/index.tsx
# Look for other input elements with spellCheck attribute
rg 'spellCheck=' -g '*.tsx' -g '*.jsx'
Length of output: 1446
| autoCorrect="off" | ||
| autoCapitalize="off" | ||
| autoComplete="off" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making autocorrect behavior configurable
Since this is a global/shared component, disabling autocorrect, autocapitalize, and autocomplete might not be suitable for all use cases. While it makes sense for address inputs, other fields like names or descriptions might benefit from these features.
Consider making these behaviors configurable through props:
type ValidatedInputProps = {
label: string
value: string
placeholder?: string
debounceTime?: number
validate: (value: string) => Promise<boolean>
onUpdate: (update: InputUpdate) => void
className?: string
+ disableAutoCorrect?: boolean
}
// In the input element:
- autoCorrect="off"
- autoCapitalize="off"
- autoComplete="off"
+ autoCorrect={disableAutoCorrect ? "off" : "on"}
+ autoCapitalize={disableAutoCorrect ? "off" : "on"}
+ autoComplete={disableAutoCorrect ? "off" : "on"}Committable suggestion skipped: line range outside the PR's diff.
…_var [TASK-6301] refactor: use base url env var
jjramirezn
changed the title
| autoCorrect="off" | ||
| autoCapitalize="off" | ||
| autoComplete="off" |
There was a problem hiding this comment.
Q, blocking: @jjramirezn why do we need this in both RecipientInput and ValidatedInput?
There was a problem hiding this comment.
yes, recipient input also will contain addresses and ENS although not validated the issue can be present there too
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
.env.example(1 hunks)src/app/claim/page.tsx(1 hunks)src/app/request/pay/page.tsx(1 hunks)src/components/Global/ImageGeneration/LinkPreview.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .env.example
🔇 Additional comments (1)
src/components/Global/ImageGeneration/LinkPreview.tsx (1)
38-38: Verify environment variable configuration
The change simplifies the URL construction by using NEXT_PUBLIC_BASE_URL, but we should ensure this environment variable is properly configured across all environments.
✅ Verification successful
Environment variable and asset configuration verified
The environment setup is properly configured:
NEXT_PUBLIC_BASE_URLis defined in.env.examplewith a default value- The
preview-bg.pngasset exists in the correct location (public/preview-bg.png) - The environment variable is consistently used across multiple files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for environment variable usage and configuration
# Test 1: Look for other usages of NEXT_PUBLIC_BASE_URL
rg "NEXT_PUBLIC_BASE_URL"
# Test 2: Check if the variable is defined in environment configuration files
fd -e env -e env.example -e env.local -e env.development -e env.production --exec cat {}
Length of output: 405
Script:
#!/bin/bash
# Let's check for the preview-bg.png file and any related image assets
fd preview-bg.png
# Check the repository root for common environment configuration locations
ls -la .env* 2>/dev/null || true
ls -la config/* 2>/dev/null || true
# Look for documentation about environment setup
rg -i "environment|env" README.md
Length of output: 186
Script:
#!/bin/bash
# Let's check the contents of .env.example to verify NEXT_PUBLIC_BASE_URL configuration
cat .env.example
# Also check if preview-bg.png exists in public directory specifically
ls -la public/preview-bg.png 2>/dev/null || true
Length of output: 765
| if (uuid) { | ||
| let host = headers().get('host') || 'peanut.to' | ||
| host = `${process.env.NODE_ENV === 'development' ? 'http://' : 'https://'}${host}` | ||
| const host = process.env.NEXT_PUBLIC_BASE_URL! |
There was a problem hiding this comment.
💡 Codebase verification
Add proper error handling for the environment variable across all usage locations
The non-null assertion (!) on NEXT_PUBLIC_BASE_URL is used in multiple files without proper error handling. This could lead to runtime errors if the environment variable is not set. The issue affects:
- src/app/request/pay/page.tsx
- src/app/claim/page.tsx
- src/components/Global/ImageGeneration/LinkPreview.tsx
Since no .env files are present in the repository (as expected, since they typically contain sensitive data), it's crucial to:
- Add proper error handling in all affected files
- Document the required environment variable in project documentation
const host = process.env.NEXT_PUBLIC_BASE_URL
if (!host) {
throw new Error('NEXT_PUBLIC_BASE_URL environment variable is not set')
}🔗 Analysis chain
Add proper error handling for the environment variable.
The non-null assertion (!) on NEXT_PUBLIC_BASE_URL could lead to runtime errors if the environment variable is not set. Consider adding proper error handling:
- const host = process.env.NEXT_PUBLIC_BASE_URL!
+ const host = process.env.NEXT_PUBLIC_BASE_URL
+ if (!host) {
+ throw new Error('NEXT_PUBLIC_BASE_URL environment variable is not set')
+ }Let's verify the environment variable setup:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for environment variable usage and configuration
# Check if NEXT_PUBLIC_BASE_URL is defined in environment files
echo "Checking .env* files for NEXT_PUBLIC_BASE_URL..."
fd -t f '^\.env' -x grep -l "NEXT_PUBLIC_BASE_URL"
# Check if there are other files potentially using this environment variable
echo "Checking for other usages of NEXT_PUBLIC_BASE_URL..."
rg "NEXT_PUBLIC_BASE_URL" --type ts --type tsx
Length of output: 555
Script:
#!/bin/bash
# Description: Check for environment variable usage with corrected commands
# Check if NEXT_PUBLIC_BASE_URL is defined in environment files (including hidden)
echo "Checking .env* files for NEXT_PUBLIC_BASE_URL..."
fd -H -t f '^\.env' -x grep -l "NEXT_PUBLIC_BASE_URL"
# Check for other usages of NEXT_PUBLIC_BASE_URL using ripgrep
echo "Checking for other usages of NEXT_PUBLIC_BASE_URL..."
rg "NEXT_PUBLIC_BASE_URL" -t typescript -t javascriptreact -t typescriptreact
Length of output: 388
Script:
#!/bin/bash
# Description: Simple search for environment variable usage
# Search for any file containing NEXT_PUBLIC_BASE_URL
echo "Searching for NEXT_PUBLIC_BASE_URL usage..."
rg "NEXT_PUBLIC_BASE_URL" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts
# List all potential environment files
echo "Listing environment files..."
fd -H '\.env'
Length of output: 557
|
|
||
| let host = headers().get('host') || 'peanut.to' | ||
| host = `${process.env.NODE_ENV === 'development' ? 'http://' : 'https://'}${host}` | ||
| const host = process.env.NEXT_PUBLIC_BASE_URL! |
There was a problem hiding this comment.
Add validation for the environment variable
The non-null assertion operator (!) assumes NEXT_PUBLIC_BASE_URL will always be defined, which could lead to runtime errors if the environment variable is missing.
Consider adding validation:
- const host = process.env.NEXT_PUBLIC_BASE_URL!
+ const host = process.env.NEXT_PUBLIC_BASE_URL
+ if (!host) {
+ throw new Error('NEXT_PUBLIC_BASE_URL environment variable is not defined')
+ }📝 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 host = process.env.NEXT_PUBLIC_BASE_URL! | |
| const host = process.env.NEXT_PUBLIC_BASE_URL | |
| if (!host) { | |
| throw new Error('NEXT_PUBLIC_BASE_URL environment variable is not defined') | |
| } |
[TASK-6981] chore: main into develop
No description provided.