Skip to content

[TASK-6981] (main) fix: disable autocorrect on address input #533

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

Merged
merged 7 commits into from
Nov 18, 2024
Merged

[TASK-6981] (main) fix: disable autocorrect on address input #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
21cdaca
refactor: use base url env var
jjramirezn Nov 16, 2024
cbe99b3
fix: disable autocorrect on address input
jjramirezn Nov 18, 2024
2be48e3
Merge pull request #532 from peanutprotocol/fix/autocorrect-on-addres…
jjramirezn Nov 18, 2024
b07a8ca
Merge branch 'main' into chore/main-to-develop
jjramirezn Nov 18, 2024
f7a0b64
fix(validatedInput): default autocomplete to off
jjramirezn Nov 18, 2024
1dc2885
Merge pull request #530 from peanutprotocol/refactor/use_base_url_env…
Hugo0 Nov 18, 2024
36725fc
Merge pull request #534 from peanutprotocol/chore/main-to-develop
jjramirezn Nov 18, 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
1 change: 1 addition & 0 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export NEXT_PUBLIC_PEANUT_ACCESS_CODE=""
export NEXT_PUBLIC_GA_KEY=""
export NEXT_PUBLIC_WC_PROJECT_ID=""
export NEXT_PUBLIC_SENTRY_DSN=""
export NEXT_PUBLIC_BASE_URL="https://peanut.to"
export MOBULA_API_KEY=""
export NEXT_PUBLIC_RECAPTCHA_SITE_KEY=""
export DISCORD_WEBHOOK_URL=""
Expand Down
4 changes: 1 addition & 3 deletions src/app/claim/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import { Claim } from '@/components'
import Layout from '@/components/Global/Layout'
import { Metadata } from 'next'
import { headers } from 'next/headers'
import { getLinkDetails } from '@squirrel-labs/peanut-sdk'
import * as utils from '@/utils'

Expand Down Expand Up @@ -33,8 +32,7 @@ function createURL(host: string, searchParams: { [key: string]: string | string[
export async function generateMetadata({ searchParams }: Props): Promise<Metadata> {
let title = 'Claim your tokens!'

let host = headers().get('host') || 'peanut.to'
host = `${process.env.NODE_ENV === 'development' ? 'http://' : 'https://'}${host}`
const host = process.env.NEXT_PUBLIC_BASE_URL!
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

Suggested change
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')
}

let linkDetails = undefined
try {
const url = createURL(host, searchParams)
Expand Down
4 changes: 1 addition & 3 deletions src/app/request/pay/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Metadata } from 'next'
import { peanut } from '@squirrel-labs/peanut-sdk'
import Layout from '@/components/Global/Layout'
import { PayRequestLink } from '@/components'
import { headers } from 'next/headers'
import { PreviewType } from '@/components/Global/ImageGeneration/LinkPreview'
import { formatAmount, printableAddress } from '@/utils'

Expand Down Expand Up @@ -34,8 +33,7 @@ export async function generateMetadata({ searchParams }: Props): Promise<Metadat
let previewUrl = '/metadata-img.jpg'
const uuid = searchParams.id ? (Array.isArray(searchParams.id) ? searchParams.id[0] : searchParams.id) : undefined
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!
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

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:

  1. Add proper error handling in all affected files
  2. 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

try {
const linkDetails = await peanut.getRequestLinkDetails({
uuid,
Expand Down
5 changes: 1 addition & 4 deletions src/components/Global/ImageGeneration/LinkPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as consts from '@/constants'
import * as utils from '@/utils'
import { headers } from 'next/headers'

export enum PreviewType {
CLAIM = 'claim',
Expand Down Expand Up @@ -36,9 +35,7 @@ export function LinkPreviewImg({
?.tokens.find((token) => utils.areTokenAddressesEqual(token.address, tokenAddress))?.logoURI
const chainImage = consts.supportedPeanutChains.find((chain) => chain.chainId === chainId)?.icon.url

let host = headers().get('host') || 'peanut.to'
host = `${process.env.NODE_ENV === 'development' ? 'http://' : 'https://'}${host}`
const previewBg = `${host}/preview-bg.png`
const previewBg = `${process.env.NEXT_PUBLIC_BASE_URL}/preview-bg.png`
return (
<div
style={{
Expand Down
3 changes: 3 additions & 0 deletions src/components/Global/RecipientInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ const RecipientInput = ({ placeholder, value, setValue, onEnter }: RecipientInpu
type="text"
placeholder={placeholder}
spellCheck="false"
autoCorrect="off"
autoCapitalize="off"
autoComplete="off"
Comment on lines +20 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 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.tsx
  • src/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

Comment on lines +20 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

Q, blocking: @jjramirezn why do we need this in both RecipientInput and ValidatedInput?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, recipient input also will contain addresses and ENS although not validated the issue can be present there too

value={value}
onChange={(e) => setValue(e.target.value)}
onKeyDown={(e) => {
Expand Down
4 changes: 3 additions & 1 deletion src/components/Global/ValidatedInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ const ValidatedInput = ({
dark:bg-n-1 dark:text-white dark:placeholder:text-white/75"
placeholder={placeholder}
spellCheck="false"
autoComplete={autoComplete || 'iban'}
autoComplete={autoComplete || 'off'}
autoCorrect="off"
autoCapitalize="off"
name={name}
list={suggestions ? listId.current : undefined}
style={{
Expand Down