-
Notifications
You must be signed in to change notification settings - Fork 84
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
fix: reactive chain and token properties from config #294
Conversation
Hey! This is your new endopint: https://876515f8.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://fdbec9c7.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://6d73ec0b.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://bd3a4f61.widget-spikesetti.pages.dev |
packages/widget/src/components/SendToWallet/SendToWalletButton.tsx
Outdated
Show resolved
Hide resolved
Hey! This is your new endopint: https://fbae9b7a.widget-spikesetti.pages.dev |
packages/widget/src/components/SendToWallet/SendToWalletButton.tsx
Outdated
Show resolved
Hide resolved
Hey! This is your new endopint: https://6459e081.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://6c0faad9.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://a93c34d0.widget-spikesetti.pages.dev |
export const FormStoreProvider: React.FC<FormStoreProviderProps> = ({ | ||
children, | ||
formRef, |
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.
Maybe we should wrap this in forwardRef
instead of passing formRef as a prop?
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 think that only applies if you want to use 'ref' as the prop name - I think keeping this as formRef is actually a good thing here. I always think that ref is generally used to refer to DOM elements as it mosts common use. Also come React 19 we are going to have to stop using forwardRef anyways
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.
Yeah, in React 19 we will have ref prop without the need to wrap the component in forwardRef, however to use ref prop now it is still required 😄 Agree, ref is often used for DOM elements and making a component controlled with ref is also a common use-case and we use useImperativeHandle
for that.
packages/widget/src/types/widget.ts
Outdated
export type FormFunctions = | ||
| { | ||
setFieldValue: SetFieldValueFunc; | ||
} | ||
| undefined; |
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.
Why this can be undefined?
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.
Because the initial state of the useRef will be undefined before the functions are added to it. That type is essentially use like this
const formRef = useRef<FormFunctions>()
formRef is passed to the LiFiWidget, without undefined stated in the type you get a type error when first passing the formRef it to the Widget as is value is initially undefined.
<LiFiWidget
...
formRef={formRef}
...
/>
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'll change this to
export type FormState = {
setFieldValue: SetFieldValueFunction;
};
And point of use should change to
const formRef = useRef<FormState>(null)
That seems cleaner
Hey! This is your new endopint: https://a5c818a6.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://4786a963.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://aa6c81fe.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://cc30e70f.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://bc13535d.widget-spikesetti.pages.dev |
Hey! This is your new endopint: https://e9b692b3.widget-spikesetti.pages.dev |
Jira: LF-9902
How to set up testing in the playground
Some tooling has been developed to test this functionality in the playground.
To access it
There are two ways you can update the form values in the Widget
Note: Config overrides URL params
Important point from this PR - Config values override URL parameters.
So you need to ensure that on first page load non of the form values that can be set in the URL are featured in the config that the Widget initialises.
So
fromAmount
,fromChain
,fromToken
,toAddress
,toChain
,toToken
andtoAmount
should not be set to allow the URL to perform the initial set up.On first page load if you have both values in the config and the url then the url search params will be rewritten to match the config values
Updating form values via the config
Once widget has initialised and has full rendered you can update the form values in the Widget by updating the config.
To perform an update you should only include the form values in the config that you want to change and ensure this is passed to the Widget.
So if you want to change the fromChain and fromToken and nothing else you should include only include those values
You also need to set the formUpdateKey. This should be a unique randomly generated key. Using this key ensures that the form values are updated in the widget - essentially forcing an update in the Widget even if the config is otherwise unchanged. This can avoid some edge case issue that might arise when setting values in the Widget via a mix of config and user actions via the Widgets UI.
So your new config might look like this
You can also reset the form values and their fields to an empty state using undefined
This example resets only the fromChain and fromToken fields
Note that
undefined
as a value is used to make the Widget reset to an empty form field for that property. The absence property rom the config object means they will remain unchanged.Updating form values via the form ref
This method provides developers using the Widget a way to set the form values directly in Widget using a function call. This approach is more imperative and might be better suited to some situations. By passing a ref object to the widget you can access a function to set values directly in the form.
For example
Once initialised you can use the ref as follows
I think this currently update the url and the widget fields as expected.
To set fromChain and fromToken
To reset fromChain and fromToken
To set fromAmount
to reset fromAmount
To set toAddress
You can also include a name to display
Or you can just set the address as a string
to reset toAddress