Skip to content
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

chore: move amount input #1893

Merged
merged 3 commits into from
Jan 29, 2025
Merged

chore: move amount input #1893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9262124
move to internal
brendan-defi Jan 28, 2025
7bea27c
add tests, remove dupe files
brendan-defi Jan 28, 2025
f2ff62d
fix lints
brendan-defi Jan 28, 2025
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
2 changes: 1 addition & 1 deletion src/fund/components/FundCard.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { FundCard } from './FundCard';
import { FundCardProvider, useFundContext } from './FundCardProvider';

const mockUpdateInputWidth = vi.fn();
vi.mock('../hooks/useInputResize', () => ({
vi.mock('../../internal/hooks/useInputResize', () => ({
useInputResize: () => mockUpdateInputWidth,
}));

Expand Down
6 changes: 3 additions & 3 deletions src/fund/components/FundCardAmountInput.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('FundCardAmountInput', () => {
);
});

const container = screen.getByTestId('ockFundCardAmountInputContainer');
const container = screen.getByTestId('ockAmountInputContainer');
expect(container).toHaveClass('custom-class');
});

Expand Down Expand Up @@ -248,7 +248,7 @@ describe('FundCardAmountInput', () => {
callback([
{
contentRect: { width: 300 },
target: screen.getByTestId('ockFundCardAmountInputContainer'),
target: screen.getByTestId('ockAmountInputContainer'),
},
]);
return {
Expand All @@ -265,7 +265,7 @@ describe('FundCardAmountInput', () => {
);

const input = screen.getByTestId('ockTextInput_Input');
const container = screen.getByTestId('ockFundCardAmountInputContainer');
const container = screen.getByTestId('ockAmountInputContainer');

// Mock getBoundingClientRect for container and currency label
Object.defineProperty(container, 'getBoundingClientRect', {
Expand Down
125 changes: 12 additions & 113 deletions src/fund/components/FundCardAmountInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import { TextInput } from '@/internal/components/TextInput';
import { useAmountInput } from '@/internal/hooks/useAmountInput';
import { isValidAmount } from '@/internal/utils/isValidAmount';
import { useCallback, useEffect, useRef } from 'react';
import { cn, text } from '../../styles/theme';
import { useInputResize } from '../hooks/useInputResize';
import { AmountInput } from '@/internal/components/amount-input/AmountInput';
import type { FundCardAmountInputPropsReact } from '../types';
import { FundCardCurrencyLabel } from './FundCardCurrencyLabel';
import { useFundContext } from './FundCardProvider';

export const FundCardAmountInput = ({
Expand All @@ -22,113 +16,18 @@ export const FundCardAmountInput = ({
setFundAmountCrypto,
} = useFundContext();

const currencyOrAsset = selectedInputType === 'fiat' ? currency : asset;

const containerRef = useRef<HTMLDivElement>(null);
const inputRef = useRef<HTMLInputElement>(null);
const hiddenSpanRef = useRef<HTMLSpanElement>(null);
const currencySpanRef = useRef<HTMLSpanElement>(null);

const value =
selectedInputType === 'fiat' ? fundAmountFiat : fundAmountCrypto;

const updateInputWidth = useInputResize(
containerRef,
inputRef,
hiddenSpanRef,
currencySpanRef,
);

const { handleChange } = useAmountInput({
setFiatAmount: setFundAmountFiat,
setCryptoAmount: setFundAmountCrypto,
selectedInputType,
exchangeRate: String(exchangeRate),
});

const handleAmountChange = useCallback(
(value: string) => {
handleChange(value, () => {
if (inputRef.current) {
inputRef.current.focus();
}
});
},
[handleChange],
);

// biome-ignore lint/correctness/useExhaustiveDependencies: When value changes, we want to update the input width
useEffect(() => {
updateInputWidth();
}, [value, updateInputWidth]);

const selectedInputTypeRef = useRef(selectedInputType);

useEffect(() => {
/**
* We need to focus the input when the input type changes
* but not on the initial render.
*/
if (selectedInputTypeRef.current !== selectedInputType) {
selectedInputTypeRef.current = selectedInputType;
handleFocusInput();
}
}, [selectedInputType]);

const handleFocusInput = () => {
if (inputRef.current) {
inputRef.current.focus();
}
};

return (
<div
ref={containerRef}
data-testid="ockFundCardAmountInputContainer"
className={cn('flex cursor-text pt-6 pb-4', className)}
>
<div className="flex h-14">
<TextInput
className={cn(
text.body,
'border-none bg-transparent',
'text-6xl leading-none outline-none',
'[appearance:textfield]',
'[&::-webkit-inner-spin-button]:m-0 [&::-webkit-inner-spin-button]:appearance-none',
'[&::-webkit-outer-spin-button]:m-0 [&::-webkit-outer-spin-button]:appearance-none',
)}
value={value}
onChange={handleAmountChange}
inputValidator={isValidAmount}
ref={inputRef}
inputMode="decimal"
placeholder="0"
/>

<FundCardCurrencyLabel ref={currencySpanRef} label={currencyOrAsset} />
</div>

{/* Hidden span for measuring text width
Without this span the input field would not adjust its width based on the text width and would look like this:
[0.12--------Empty Space-------][ETH] - As you can see the currency symbol is far away from the inputed value

With this span we can measure the width of the text in the input field and set the width of the input field to match the text width
[0.12][ETH] - Now the currency symbol is displayed next to the input field
*/}
<span
data-testid="ockHiddenSpan"
ref={hiddenSpanRef}
className={cn(
text.body,
'border-none bg-transparent',
'text-6xl leading-none outline-none',
'pointer-events-none absolute whitespace-nowrap opacity-0',
'left-[-9999px]', // Hide the span from the DOM
)}
>
{value ? `${value}.` : '0.'}
</span>
</div>
<AmountInput
fiatAmount={fundAmountFiat}
cryptoAmount={fundAmountCrypto}
asset={asset}
selectedInputType={selectedInputType}
currency={currency}
className={className}
setFiatAmount={setFundAmountFiat}
setCryptoAmount={setFundAmountCrypto}
exchangeRate={String(exchangeRate)}
/>
);
};

Expand Down
70 changes: 12 additions & 58 deletions src/fund/components/FundCardAmountInputTypeSwitch.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import { formatFiatAmount } from '@/internal/utils/formatFiatAmount';
import { truncateDecimalPlaces } from '@/internal/utils/truncateDecimalPlaces';
import { useCallback, useMemo } from 'react';
import { Skeleton } from '../../internal/components/Skeleton';
import { useIcon } from '../../internal/hooks/useIcon';
import { cn, pressable, text } from '../../styles/theme';
import { AmountInputTypeSwitch } from '@/internal/components/amount-input/AmountInputTypeSwitch';
import type { FundCardAmountInputTypeSwitchPropsReact } from '../types';
import { useFundContext } from './FundCardProvider';

Expand All @@ -21,59 +16,18 @@ export const FundCardAmountInputTypeSwitch = ({
currency,
} = useFundContext();

const iconSvg = useIcon({ icon: 'toggle' });

const handleToggle = useCallback(() => {
setSelectedInputType(selectedInputType === 'fiat' ? 'crypto' : 'fiat');
}, [selectedInputType, setSelectedInputType]);

const formatCrypto = useCallback(
(amount: string) => {
return `${truncateDecimalPlaces(amount || '0', 8)} ${asset}`;
},
[asset],
);

const amountLine = useMemo(() => {
return (
<span data-testid="ockAmountLine" className={cn(text.label1)}>
{selectedInputType === 'fiat'
? formatCrypto(fundAmountCrypto)
: formatFiatAmount({
amount: fundAmountFiat,
currency: currency,
minimumFractionDigits: 0,
})}
</span>
);
}, [
fundAmountCrypto,
fundAmountFiat,
selectedInputType,
formatCrypto,
currency,
]);

if (exchangeRateLoading || !exchangeRate) {
return <Skeleton className="h-[1.625rem]" />;
}

return (
<div className={cn('flex items-center', className)}>
<button
type="button"
aria-label="amount type switch"
className={cn(
pressable.default,
'mr-1 rounded-full p-1 opacity-50 transition-opacity hover:opacity-100',
)}
data-testid="ockAmountTypeSwitch"
onClick={handleToggle}
>
<div className="h-[1.125rem] w-[1.125rem]">{iconSvg}</div>
</button>
<div className="w-full truncate">{amountLine}</div>
</div>
<AmountInputTypeSwitch
selectedInputType={selectedInputType}
setSelectedInputType={setSelectedInputType}
asset={asset}
fiatAmount={fundAmountFiat}
cryptoAmount={fundAmountCrypto}
exchangeRate={exchangeRate}
exchangeRateLoading={exchangeRateLoading}
currency={currency}
className={className}
/>
);
};

Expand Down
23 changes: 0 additions & 23 deletions src/fund/components/FundCardCurrencyLabel.tsx
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions src/fund/components/FundCardProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ type FundCardContextType = {
setFundAmountFiat: (amount: string) => void;
fundAmountCrypto: string;
setFundAmountCrypto: (amount: string) => void;
exchangeRate?: number;
exchangeRate: number;
setExchangeRate: (exchangeRate: number) => void;
exchangeRateLoading?: boolean;
exchangeRateLoading: boolean;
setExchangeRateLoading: (loading: boolean) => void;
submitButtonState: FundButtonStateReact;
setSubmitButtonState: (state: FundButtonStateReact) => void;
Expand Down
11 changes: 11 additions & 0 deletions src/internal/components/TextInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@ type TextInputReact = {
inputMode?: InputHTMLAttributes<HTMLInputElement>['inputMode'];
onBlur?: () => void;
onChange: (s: string) => void;
onFocus?: (e: React.FocusEvent<HTMLInputElement>) => void;
placeholder: string;
setValue?: (s: string) => void;
value: string;
inputValidator?: (s: string) => boolean;
/** autocomplete attribute handles browser autocomplete, defaults to 'off' */
autoComplete?: string;
/** data-1p-ignore attribute handles password manager autocomplete, defaults to true */
'data-1p-ignore'?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there ever a case where we don't want to ignore it? I'd argue for just removing it completely and setting our inputs to always ignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think it's probably right that we always want to ignore, but since this is internal i figured i'd leave us the option. wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's internal + not planned to be used I'd rather remove it since it keeps the props more concise, but I won't block on this issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, i'll create ticket to remove

};

export const TextInput = forwardRef<HTMLInputElement, TextInputReact>(
Expand All @@ -30,11 +35,14 @@ export const TextInput = forwardRef<HTMLInputElement, TextInputReact>(
disabled = false,
onBlur,
onChange,
onFocus,
placeholder,
setValue,
inputMode,
value,
inputValidator = () => true,
autoComplete = 'off',
'data-1p-ignore': data1pIgnore = true,
},
ref,
) => {
Expand Down Expand Up @@ -70,7 +78,10 @@ export const TextInput = forwardRef<HTMLInputElement, TextInputReact>(
value={value}
onBlur={onBlur}
onChange={handleChange}
onFocus={onFocus}
disabled={disabled}
autoComplete={autoComplete}
data-1p-ignore={data1pIgnore}
/>
);
},
Expand Down
Loading
Loading