-
Notifications
You must be signed in to change notification settings - Fork 193
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
Support fiat input in dual currency field #3089
Open
riccardobl
wants to merge
23
commits into
getAlby:master
Choose a base branch
from
riccardobl:fiatinput
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
8c1567b
feat: dual currency input supports fiat input
riccardobl 87a8edc
feat: prefix dual currency input with main symbol, add currency to in…
riccardobl 3bffba9
fix: amount in Satoshis -> amount in sats
riccardobl 29c49b4
fix: attempt at making unit tests happy
riccardobl 90512cf
Merge remote-tracking branch 'upstream/master' into fiatinput
pavanjoshi914 3f7f107
Merge remote-tracking branch 'upstream/master' into fiatinput
pavanjoshi914 96635c8
Merge branch 'fiatinput' of https://github.com/getAlby/lightning-brow…
pavanjoshi914 0394bb7
fix: fix some unit tests
riccardobl 2eb1020
fix: dual currency input event wrapping
riccardobl 5a214c9
fix: more unit tests fixing
riccardobl 2e2e329
fix: currency toggle issue for default values, improve some naming
riccardobl ea1954f
fix: input value
riccardobl 9cfa078
Merge remote-tracking branch 'upstream/master' into fiatinput
pavanjoshi914 278acaf
fix: naming, empty input, do not change component to uncontrolled whe…
riccardobl 49bb31c
fix: fixes and improvements
riccardobl fd36df6
fix: light theme
riccardobl 269f8a3
Merge remote-tracking branch 'upstream/master' into fiatinput
pavanjoshi914 5b75f43
fix: dualcurrency component test
pavanjoshi914 9ff8337
fix: clone target to avoid side-effects when changing value
riccardobl 564f79e
fix: tests and implementation
riccardobl ba7cb69
Merge branch 'master' into fiatinput
bumi 0cdba7b
fix: allow 4 decimals in fiat input and add some comments
riccardobl c49b925
fix: preset workaround
riccardobl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,35 @@ | ||
import { useEffect, useRef } from "react"; | ||
import { useCallback, useEffect, useRef, useState } from "react"; | ||
import { useTranslation } from "react-i18next"; | ||
import { useAccount } from "~/app/context/AccountContext"; | ||
import { useSettings } from "~/app/context/SettingsContext"; | ||
import { classNames } from "~/app/utils"; | ||
|
||
import { RangeLabel } from "./rangeLabel"; | ||
|
||
export type DualCurrencyFieldChangeEvent = | ||
React.ChangeEvent<HTMLInputElement> & { | ||
target: HTMLInputElement & { | ||
valueInFiat: number; | ||
formattedValueInFiat: string; | ||
valueInSats: number; | ||
formattedValueInSats: string; | ||
}; | ||
}; | ||
|
||
export type Props = { | ||
suffix?: string; | ||
endAdornment?: React.ReactNode; | ||
fiatValue: string; | ||
label: string; | ||
hint?: string; | ||
amountExceeded?: boolean; | ||
rangeExceeded?: boolean; | ||
baseToAltRate?: number; | ||
showFiat?: boolean; | ||
onChange?: (e: DualCurrencyFieldChangeEvent) => void; | ||
}; | ||
|
||
export default function DualCurrencyField({ | ||
label, | ||
fiatValue, | ||
showFiat = true, | ||
id, | ||
placeholder, | ||
required = false, | ||
|
@@ -38,10 +51,149 @@ export default function DualCurrencyField({ | |
rangeExceeded, | ||
}: React.InputHTMLAttributes<HTMLInputElement> & Props) { | ||
const { t: tCommon } = useTranslation("common"); | ||
const { | ||
getFormattedInCurrency, | ||
getCurrencyRate, | ||
getCurrencySymbol, | ||
settings, | ||
} = useSettings(); | ||
const { account } = useAccount(); | ||
|
||
const inputEl = useRef<HTMLInputElement>(null); | ||
const outerStyles = | ||
"rounded-md border border-gray-300 dark:border-gray-800 bg-white dark:bg-black transition duration-300"; | ||
|
||
const initialized = useRef(false); | ||
const [useFiatAsMain, _setUseFiatAsMain] = useState(false); | ||
const [altFormattedValue, setAltFormattedValue] = useState(""); | ||
const [minValue, setMinValue] = useState(min); | ||
const [maxValue, setMaxValue] = useState(max); | ||
const [inputValue, setInputValue] = useState(value || 0); | ||
const [inputPrefix, setInputPrefix] = useState(""); | ||
riccardobl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const [inputPlaceHolder, setInputPlaceHolder] = useState(placeholder || ""); | ||
|
||
const convertValues = useCallback( | ||
async (inputValue: number, inputInFiat: boolean) => { | ||
const userCurrency = settings?.currency || "BTC"; | ||
let valueInSats = 0; | ||
let valueInFiat = 0; | ||
const rate = await getCurrencyRate(); | ||
|
||
if (inputInFiat) { | ||
valueInFiat = Number(inputValue); | ||
valueInSats = Math.round(valueInFiat / rate); | ||
} else { | ||
valueInSats = Number(inputValue); | ||
valueInFiat = Math.round(valueInSats * rate * 100) / 100.0; | ||
} | ||
|
||
const formattedSats = getFormattedInCurrency(valueInSats, "BTC"); | ||
const formattedFiat = getFormattedInCurrency(valueInFiat, userCurrency); | ||
|
||
return { | ||
valueInSats, | ||
formattedSats, | ||
valueInFiat, | ||
formattedFiat, | ||
}; | ||
}, | ||
[getCurrencyRate, getFormattedInCurrency, settings] | ||
); | ||
|
||
const setUseFiatAsMain = useCallback( | ||
async (v: boolean) => { | ||
if (!showFiat) v = false; | ||
riccardobl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const userCurrency = settings?.currency || "BTC"; | ||
const rate = await getCurrencyRate(); | ||
|
||
if (min) { | ||
setMinValue( | ||
v ? (Math.round(Number(min) * rate * 100) / 100.0).toString() : min | ||
); | ||
} | ||
|
||
if (max) { | ||
setMaxValue( | ||
v ? (Math.round(Number(max) * rate * 100) / 100.0).toString() : max | ||
); | ||
} | ||
|
||
const newValue = v | ||
? Math.round(Number(inputValue) * rate * 100) / 100.0 | ||
: Math.round(Number(inputValue) / rate); | ||
|
||
_setUseFiatAsMain(v); | ||
setInputValue(newValue); | ||
setInputPrefix(getCurrencySymbol(v ? userCurrency : "BTC")); | ||
if (!placeholder) { | ||
setInputPlaceHolder( | ||
tCommon("amount_placeholder", { | ||
currency: v ? userCurrency : "sats", | ||
}) | ||
); | ||
} | ||
}, | ||
[ | ||
settings, | ||
showFiat, | ||
getCurrencyRate, | ||
inputValue, | ||
min, | ||
max, | ||
tCommon, | ||
getCurrencySymbol, | ||
placeholder, | ||
] | ||
); | ||
|
||
const swapCurrencies = () => { | ||
setUseFiatAsMain(!useFiatAsMain); | ||
}; | ||
|
||
const onChangeWrapper = useCallback( | ||
async (e: React.ChangeEvent<HTMLInputElement>) => { | ||
setInputValue(e.target.value); | ||
|
||
if (onChange) { | ||
const value = Number(e.target.value); | ||
const { valueInSats, formattedSats, valueInFiat, formattedFiat } = | ||
await convertValues(value, useFiatAsMain); | ||
const wrappedEvent: DualCurrencyFieldChangeEvent = | ||
e as DualCurrencyFieldChangeEvent; | ||
wrappedEvent.target.value = valueInSats.toString(); | ||
wrappedEvent.target.valueInFiat = valueInFiat; | ||
wrappedEvent.target.formattedValueInFiat = formattedFiat; | ||
wrappedEvent.target.valueInSats = valueInSats; | ||
wrappedEvent.target.formattedValueInSats = formattedSats; | ||
onChange(wrappedEvent); | ||
} | ||
}, | ||
[onChange, useFiatAsMain, convertValues] | ||
); | ||
|
||
// default to fiat when account currency is set to anything other than BTC | ||
useEffect(() => { | ||
if (!initialized.current) { | ||
if (account?.currency && account?.currency !== "BTC") { | ||
setUseFiatAsMain(true); | ||
} | ||
initialized.current = true; | ||
} | ||
}, [account?.currency, setUseFiatAsMain]); | ||
|
||
// update alt value | ||
useEffect(() => { | ||
(async () => { | ||
if (showFiat) { | ||
const { formattedSats, formattedFiat } = await convertValues( | ||
Number(inputValue), | ||
useFiatAsMain | ||
); | ||
setAltFormattedValue(useFiatAsMain ? formattedSats : formattedFiat); | ||
} | ||
})(); | ||
}, [useFiatAsMain, inputValue, convertValues, showFiat]); | ||
|
||
const inputNode = ( | ||
<input | ||
ref={inputEl} | ||
|
@@ -53,19 +205,20 @@ export default function DualCurrencyField({ | |
"block w-full placeholder-gray-500 dark:placeholder-gray-600 dark:text-white ", | ||
"px-0 border-0 focus:ring-0 bg-transparent" | ||
)} | ||
placeholder={placeholder} | ||
placeholder={inputPlaceHolder} | ||
required={required} | ||
pattern={pattern} | ||
title={title} | ||
onChange={onChange} | ||
onChange={onChangeWrapper} | ||
onFocus={onFocus} | ||
onBlur={onBlur} | ||
value={value} | ||
value={inputValue ? inputValue : undefined} | ||
autoFocus={autoFocus} | ||
autoComplete={autoComplete} | ||
disabled={disabled} | ||
min={min} | ||
max={max} | ||
min={minValue} | ||
max={maxValue} | ||
step={useFiatAsMain ? "0.01" : "1"} | ||
/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if we keep step value upto 4 decimal places. so that user can enter values for 10 20 sats in USD as well (it has no major use but just to keep it fully flexible) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
); | ||
|
||
|
@@ -90,14 +243,15 @@ export default function DualCurrencyField({ | |
> | ||
{label} | ||
</label> | ||
{(min || max) && ( | ||
{(minValue || maxValue) && ( | ||
<span | ||
className={classNames( | ||
"text-xs text-gray-700 dark:text-neutral-400", | ||
!!rangeExceeded && "text-red-500 dark:text-red-500" | ||
)} | ||
> | ||
<RangeLabel min={min} max={max} /> {tCommon("sats_other")} | ||
<RangeLabel min={minValue} max={maxValue} />{" "} | ||
{useFiatAsMain ? "" : tCommon("sats_other")} | ||
</span> | ||
)} | ||
</div> | ||
|
@@ -112,17 +266,26 @@ export default function DualCurrencyField({ | |
outerStyles | ||
)} | ||
> | ||
{!!inputPrefix && ( | ||
<p className="helper text-gray-500 z-1 pr-2" onClick={swapCurrencies}> | ||
{inputPrefix} | ||
</p> | ||
)} | ||
|
||
{inputNode} | ||
|
||
{!!fiatValue && ( | ||
<p className="helper text-gray-500 z-1 pointer-events-none"> | ||
~{fiatValue} | ||
{!!altFormattedValue && ( | ||
<p | ||
className="helper whitespace-nowrap text-gray-500 z-1" | ||
onClick={swapCurrencies} | ||
> | ||
~{altFormattedValue} | ||
</p> | ||
)} | ||
|
||
{suffix && ( | ||
<span | ||
className="flex items-center px-3 font-medium bg-white dark:bg-surface-00dp dark:text-white" | ||
className="flex items-center px-3 font-medium bg-white dark:bg-surface-00dp dark:text-white" | ||
onClick={() => { | ||
inputEl.current?.focus(); | ||
}} | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now its a good enough complexity. maybe add comments wherere necessary why such and such thing is used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some comments to the code and removed baseToAltRate since it was dead code from one of the previous iterations 👍