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

chore: add loading state to the response of useUser (initial PR, still polishing and testing) #1199

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Shenghu-Yang
Copy link
Collaborator

@Shenghu-Yang Shenghu-Yang commented Dec 10, 2024

Pull Request Checklist


PR-Codex overview

This PR enhances the useUser hook's type definitions and logic to include a loading state for users while the account is reconnecting. It improves type safety and clarity regarding the user status.

Detailed summary

  • Updated the UseUserResult type to include a loading state.
  • Added a new return object for the "reconnecting" account status in the useUser hook.
  • Ensured that the loading property is optional for certain user types.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

vercel bot commented Dec 10, 2024

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

Name Status Preview Comments Updated (UTC)
aa-sdk-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 10, 2024 0:42am
aa-sdk-ui-demo ❌ Failed (Inspect) Dec 10, 2024 0:42am

Copy link

graphite-app bot commented Dec 10, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “graphite-merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@@ -35,6 +39,13 @@ export const useUser = (): UseUserResult => {
const eoaUser = useMemo(() => {
if (account.status !== "connected" && account.status !== "reconnecting") {
return null;
} else if (account.status == "reconnecting") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ [eslint] <eqeqeq> reported by reviewdog 🐶
Expected '===' and instead saw '=='.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks @moldy530 . I am brand new to ts.

change is on its way.

@rthomare
Copy link
Collaborator

It's always super helpful to include how you tested it in your PR! Feel free to add a video or screenshot that helps illistrate how the fix was made and how you ensure there weren't any regressions. If you aren't sure what or how to test the UI please ping myself, @dphilipson, @moldy530!!

@Shenghu-Yang Shenghu-Yang changed the title chore: add loading state to the response of useUser chore: add loading state to the response of useUser (initial PR, still polishing and testing) Dec 11, 2024
@Shenghu-Yang
Copy link
Collaborator Author

Yes @rthomare, just marked the PR as WIP. I was trying to get a sense of how my PR look in github, still working with @moldy530 for testing and further polishing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants