-
Notifications
You must be signed in to change notification settings - Fork 10
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
Added Don't Show Again On Deposit Dialog #1122
base: dev
Are you sure you want to change the base?
Conversation
This isn't entirely solving the problem we are going to need to use more than react state if we want this to persist across sessions Use react native async storage so we persist the check in local storage Use local storage then I'll give this a full review |
Should I proceed with using AsyncStorage for persistence? Is it already installed in the project, or would I need to add it? |
@SABITRADE its already there. Here is an example that is used to persist the price hidden state |
This comment has been minimized.
This comment has been minimized.
@youngkidwarrior you can now review again and tell me if you find anything wrong |
|
||
function CopyAddressDialog({ isOpen, onClose, onConfirm }) { | ||
function CopyAddressDialog({ isOpen, onClose, onConfirm }) { const [dontShowAgain, setDontShowAgain] = useState(false) |
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.
can you please use our biome integration to format these changes
<input | ||
type="checkbox" | ||
checked={dontShowAgain} | ||
onChange={async () => { |
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.
don't do this inline. Pop this out into a new function
<XStack justifyContent="flex-end" marginTop="$4" gap="$4" ai="center"> | ||
{/* Checkbox on the left */} | ||
<XStack ai="center" gap="$2"> | ||
<input |
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.
We have a checkbox form component that has some helpful defaults. lets use that here
|
||
function CopyAddressDialog({ isOpen, onClose, onConfirm }) { | ||
function CopyAddressDialog({ isOpen, onClose, onConfirm }) { const [dontShowAgain, setDontShowAgain] = useState(false) |
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.
we won't need this local state if we are using the checkbox form and the async storage
const savedValue = await AsyncStorage.getItem('dontShowAgain'); | ||
if (savedValue !== null) { | ||
setDontShowAgain(JSON.parse(savedValue)); | ||
setIsConfirmed(JSON.parse(savedValue)); // Auto-confirm if "Don't show again" was checked |
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.
this isn't the right way to do this. Instead we should load this state from storage in the screen component and pass it as props to skip over the dialog entirely
I added the option to mark a checkbox ✅ to agree Once on the pop-up warning message cause it's annoying