feat: keep form data in memory#1525
Conversation
necipsagiro
requested review from
gbarkhatov,
jeremy-babylonlabs,
jonybur and
jrwbabylonlab
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR implements form data persistence in memory to maintain user-entered data across tab switches and route navigation. The implementation includes tab mounting behavior changes and form state management for cross-route drafts.
- Added
FormPersistenceStateprovider for managing in-memory form drafts across routes - Modified
Tabscomponent to supportkeepMountedprop for preserving tab content in DOM - Enhanced
FeeModalto preselect fee rate options based on current fee rate
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/common/state/index.tsx | Added FormPersistenceState to the state provider list |
| src/ui/common/state/FormPersistenceState.tsx | New state provider for managing BTC and Baby stake form drafts |
| src/ui/common/page.tsx | Enabled keepMounted prop for main tabs |
| src/ui/common/components/Tabs/Tabs.tsx | Added keepMounted functionality to preserve tab content |
| src/ui/common/components/Staking/FeeModal/index.tsx | Added currentFeeRate prop and preselection logic |
| src/ui/common/components/Multistaking/MultistakingForm/StakingFeesSection.tsx | Removed redundant default fee rate setting and passed currentFeeRate to FeeModal |
| src/ui/common/components/Multistaking/MultistakingForm/MultistakingFormContent.tsx | Added BtcFormPersistence component |
| src/ui/common/components/Multistaking/MultistakingForm/BtcFormPersistence.tsx | New component for persisting BTC staking form data |
| src/ui/baby/widgets/StakingForm/index.tsx | Added BabyFormPersistence component |
| src/ui/baby/widgets/StakingForm/BabyFormPersistence.tsx | New component for persisting Baby staking form data |
| src/ui/baby/layout.tsx | Enabled keepMounted prop for baby staking tabs |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| stakingInfo?.defaultFeeRate !== undefined && | ||
| (feeRate === undefined || feeRate === "") |
There was a problem hiding this comment.
[nitpick] The condition feeRate === \"\" should use a more robust check. Consider using a utility function to check for empty/falsy values consistently across the codebase, or at minimum check for both \"\" and \"0\".
| stakingInfo?.defaultFeeRate !== undefined && | |
| (feeRate === undefined || feeRate === "") | |
| (feeRate === undefined || feeRate === "" || feeRate === "0") |
| const fee = Number(currentFeeRate); | ||
| if (!fee || !Number.isFinite(fee)) { |
There was a problem hiding this comment.
The condition !fee will be true for valid fee rate of 0, which should be allowed. Change to !Number.isFinite(fee) || fee <= 0 to properly handle zero values while rejecting invalid numbers.
| const fee = Number(currentFeeRate); | |
| if (!fee || !Number.isFinite(fee)) { | |
| if (!Number.isFinite(fee) || fee < 0) { |
gbarkhatov
left a comment
gbarkhatov
left a comment
There was a problem hiding this comment.
Approved with some comments, let's wait for @jonybur and @jeremy-babylonlabs also
| if (babyStakeDraft) { | ||
| if (babyStakeDraft.amount !== undefined) { | ||
| setValue("amount", babyStakeDraft.amount, { | ||
| shouldValidate: true, |
There was a problem hiding this comment.
Can we have comments on these values so it's easier to understand why, besides Hydrate once on mount
| if (btcStakeDraft) { | ||
| if (btcStakeDraft.finalityProviders) { | ||
| setValue("finalityProviders", btcStakeDraft.finalityProviders, { | ||
| shouldValidate: false, |
|
|
||
| import { createStateUtils } from "@/ui/common/utils/createStateUtils"; | ||
|
|
||
| export type BtcStakeDraft = { |
There was a problem hiding this comment.
Let's add comments for these types and interfaces
| } | ||
| if (babyStakeDraft.feeAmount !== undefined) { | ||
| setValue("feeAmount", babyStakeDraft.feeAmount, { | ||
| shouldValidate: true, |
There was a problem hiding this comment.
Could probably move these {shouldValidate: true, shouldDirty: false, shouldTouch: false} to a const and assign on each of these setValues. Would make the code a bit cleaner.
Actually, since we are at it, this is simply a matter of doing something like
["amount", "validatorAddress", "feeAmount"].forEach(value => {
if (babyStakeDraft[value] !== undefined) setValue(value, /*object here*/)
}
| className="flex flex-col gap-2 h-[500px]" | ||
| onSubmit={handlePreview} | ||
| > | ||
| <BabyFormPersistence /> |
There was a problem hiding this comment.
Why is this a functional component? can't this be a hook instead? Since it's returning null that is
Added
keepMountedtoTabsAdded
FormPersistenceStateprovider that wraps app for cross-route in-memory draftsFeeModalnow takescurrentFeeRateand preselects fast/medium/slow/customScreen.Recording.2025-09-02.at.16.20.15.mov
Closes: #1403