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.

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

Disclaimer #2097

Merged
merged 14 commits into from
Jan 16, 2024
Merged

Disclaimer #2097

merged 14 commits into from
Jan 16, 2024

Conversation

Robiquet
Copy link
Member

closes #1822

Copy link

vercel bot commented Dec 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2024 8:47am

Copy link
Contributor

@robhyrk robhyrk left a comment

Choose a reason for hiding this comment

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

I think there may be an issue with how the persistent atom is being saved. I did some testing and I noticed the the atom being saved in storage holds the following value after accepting the user agreement {__version: 0}. It seems like the boolean for the defaultValue is not being saved. That being said, the disclaimer functionality seems to work since it checks for the truthy value of thedisclaimerStatus object returned.

As an aside: I'm not sure if userLocation is working correctly either. I used as VPN via the US and it doesn't seem to be picking up correctly.

@Robiquet
Copy link
Member Author

I think there may be an issue with how the persistent atom is being saved. I did some testing and I noticed the the atom being saved in storage holds the following value after accepting the user agreement {__version: 0}. It seems like the boolean for the defaultValue is not being saved. That being said, the disclaimer functionality seems to work since it checks for the truthy value of thedisclaimerStatus object returned.

As an aside: I'm not sure if userLocation is working correctly either. I used as VPN via the US and it doesn't seem to be picking up correctly.

I think the disclaimerStatus is a quirk of persistentAtom I've changed it to an object to make it explicit. Regarding the location, what behaviour are you expecting? I added some debug logs to verify the location is getting picked up correctly

Copy link
Contributor

@robhyrk robhyrk left a comment

Choose a reason for hiding this comment

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

Looks good!

@Robiquet Robiquet merged commit 7d90852 into staging Jan 16, 2024
5 checks passed
@Robiquet Robiquet deleted the tr-remove-vpn branch January 16, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add disclaimer
2 participants