Skip to content

fix: normalize inputAmount by stripping leading zeros after first non… #1118

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.

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

Open
wants to merge 7 commits into
base: next
Choose a base branch
from

Conversation

nikaaru
Copy link
Member

@nikaaru nikaaru commented Apr 30, 2025

📝 Summary

  • Allows users to type raw-zero values like 0000, 0., or 0.0000 without immediate formatting
  • As soon as a non-zero digit is entered, strips leading zeros and normalizes input (e.g. 000011, 0000.010.01)
  • Enforces a minimum acceptable value of 0.00001 — input below this (e.g. 0.00000) is blocked

🔍 Changes

  • Added four regex constants in UPPER_SNAKE_CASE:

    • ALL_ZERO_REGEX – detects pure-zero strings (e.g., 0000, 0.000)
    • LEADING_ZEROS_REGEX – strips leading zeros only when followed by a digit
    • LEADING_DOT_REGEX – normalizes inputs like .50.5
    • ALLOWABLE_ZERO_INPUT_REGEX – allows partial raw-zero input like 0., 0.000
  • Updated setInputAmount:

    • Allows raw-zero phase inputs (via ALL_ZERO_REGEX + ALLOWABLE_ZERO_INPUT_REGEX)
    • Sanitizes only after the first meaningful digit using LEADING_ZEROS_REGEX and LEADING_DOT_REGEX
    • Blocks values below 0.00001 by checking parsed numeric value

🛠️ How to Test

  1. Run the app and enter each scenario from the table below.
  2. Ensure the input behaves as expected and normalization occurs only when appropriate.

✅ Some Test Scenarios

Input Expected Output Notes
00000 00000 raw-zero, allowed
0. 0. raw-zero + dot, allowed
0.0000 0.0000 up to 4 decimal zeros allowed
00001 1 sanitized
0000.01 0.01 sanitized + normalized
.5 0.5 normalized
0.00001 0.00001 meets minimum, allowed

🔄 Updates (since last review)

  • Extracted sanitize/validate logic into sanitizer.ts & validation.ts
  • Added sanitizer.test.ts & validation.test.ts with edge‑case coverage
  • Removed constants from constants.ts (ALL_ZERO_REGEX, LEADING_ZEROS_REGEX, LEADING_DOT_REGEX, ALLOWABLE_ZERO_INPUT_REGEX) — now internalized in sanitizer.ts and validation.ts.
  • Removed "minimum value" validation (no longer blocks 0.00000)
  • Refactored and extracted shared logic into new utility modules:
    sanitizer.ts: includes formatThousandsWithCommas, replaceSpacesWithDash
    validation.ts: includes isValidCurrencyFormat, isNumeric, isColorKeyOverridden
  • Added unit tests for all extracted utility functions
  • Added sanitizeInputAmount method to normalize zero values (e.g., 00000) on input blur
  • Sync URL search params with quoteStore.

🧪 Verification Steps

  • Review new files sanitizer.ts and validation.ts for correct function extraction.
  • Run a yarn test and confirm that all tests in sanitizer.test.ts and validation.test.ts pass.

Copy link
Collaborator

@yeager-eren yeager-eren left a comment

Choose a reason for hiding this comment

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

Why you didn't use zod‍‍?

@yeager-eren
Copy link
Collaborator

  1. Query string should be validated and fixed on first page load. e.g.
  2. Blur event should be used also to update and fix user's input. e.g. type 0000 and click outside the input (blur).

@nikaaru
Copy link
Member Author

nikaaru commented May 5, 2025

Why you didn't use zod‍‍?

I'm not using Zod here:

  • Because it hasn't been added to rango-client before.
  • Adding Zod now would bloat our bundle
  • We don't show the user a proper message either.
  • And also, it wasn't mentioned in the task description either.

@nikaaru
Copy link
Member Author

nikaaru commented May 5, 2025

  1. Query string should be validated and fixed on first page load. e.g.
  2. Blur event should be used also to update and fix user's input. e.g. type 0000 and click outside the input (blur).
  1. Good catch, the query string was also validated, but it wasn't truncated properly, so I fixed it.
  2. Blur's behavior is 1inch different, and also this was not mentioned in the task.

@nikaaru nikaaru requested a review from mikasackermn as a code owner May 6, 2025 14:14
Copy link
Collaborator

@yeager-eren yeager-eren left a comment

Choose a reason for hiding this comment

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

neat implementation. thanks.

Copy link
Collaborator

@yeager-eren yeager-eren left a comment

Choose a reason for hiding this comment

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

I resolved my comments. you can move it to test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants