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

ENG-4662 feat(portal,1ui,graphql): update query in create identity modal #889

Merged

Conversation

Vitalsine85
Copy link
Member

Affected Packages

Apps

  • data populator
  • portal
  • template

Packages

  • 1ui
  • api
  • graphql
  • protocol
  • sdk

Tools

  • tools

Overview

  • Swaps out the fetcher used to get multivault config with a new React Query hook.
  • Made a couple minor adjustments to ProfileCard and Trunctacular that we're made in my other create identity flow update PR. Nothing major, just noticed during testing and pretty jarring issues visually.
  • Changed Estimated Fees to Estimated Cost on the review screen as it was pretty misleading when you add an initial deposit. This is also fixed in the other PR mentioned above.
  • Added dotenv-cli to graphql dev dependencies. Was unable to run codegen without it.

Screen Captures

If applicable, add screenshots or screen captures of your changes.

Declaration

  • I hereby declare that I have abided by the rules and regulations as outlined in the CONTRIBUTING.md

Copy link

linear bot commented Nov 6, 2024

@Vitalsine85 Vitalsine85 enabled auto-merge (squash) November 6, 2024 20:00
Copy link
Member

@jonathanprozzi jonathanprozzi left a comment

Choose a reason for hiding this comment

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

Lmk about specifics with codegen issue since I want to try to avoid adding this as a dep if possible! Otherwise looks good!

@@ -33,6 +33,7 @@
},
"devDependencies": {
"@0no-co/graphqlsp": "^1.12.16",
"dotenv-cli": "^7.4.1",
Copy link
Member

Choose a reason for hiding this comment

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

Why this dependency? Curious if it was needed for something you ran into

Edit: saw codegen comment. What error? Did you run from the root? Also did you run to add any new queries? Trying to see if this is an issue we can solve without adding this as a dependency- I haven't had issues with codegen but if you saw errors lmk

Copy link
Member Author

Choose a reason for hiding this comment

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

➜  intuition-ts git:(vital/eng-4662-update-queries-in-create-identity-modal) ✗ pnpm run graphql:codegen

> @0xintuition/intuition-ts@ graphql:codegen /home/vital/Git/intuition/intuition-ts
> nx codegen @0xintuition/graphql


> nx run @0xintuition/graphql:codegen


> @0xintuition/[email protected] codegen /home/vital/Git/intuition/intuition-ts/packages/graphql
> dotenv graphql-codegen --config codegen.ts

sh: 1: dotenv: not found
 ELIFECYCLE  Command failed.

———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 NX   Ran target codegen for project @0xintuition/graphql (514ms)

   ✖  1/1 failed
   ✔  0/1 succeeded [0 read from cache]

 ELIFECYCLE  Command failed with exit code 1.

This is what I was seeing, added dotenv-cli to devDependencies based on Claudes comments. I think I could have installed it locally and it would have worked. Can remove it though if we need to. I'm assuming you must have it installed on your local machine?

minDeposit: string
}

export function useCreateConfig() {
Copy link
Member

Choose a reason for hiding this comment

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

Nice. Good idea!

@Vitalsine85 Vitalsine85 merged commit 4b5bc6f into main Nov 6, 2024
3 checks passed
@Vitalsine85 Vitalsine85 deleted the vital/eng-4662-update-queries-in-create-identity-modal branch November 6, 2024 21:45
Vitalsine85 added a commit that referenced this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants