Skip to content

feat: use subgraph to pre-populate gauge options for the safe user in ui#31

Merged
petrovska-petro merged 4 commits intofeat/safe-app-fixfrom
feat/subgraph-query-ui-test
Aug 24, 2023
Merged

feat: use subgraph to pre-populate gauge options for the safe user in ui#31
petrovska-petro merged 4 commits intofeat/safe-app-fixfrom
feat/subgraph-query-ui-test

Conversation

@petrovska-petro
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Contributor

@gosuto-inzasheru gosuto-inzasheru left a comment

Choose a reason for hiding this comment

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

looks great, works on localhost for me!

Screenshot 2023-08-23 at 09 39 27

seeing the pool name of the gauge and not the address is a great step in ux :)

this does raise a new question; what if we want to enable harvests for both?

and probably also somewhat out of scope, but i noted that even if connected to opti goerli the same gauges show up. so somewhere we should start becoming chain aware. (a subgraph will always be for a single specific chain only, for a different chain a different subgraph will be needed)

Comment thread src/components/GraphQuery.tsx Outdated
Comment thread src/queries/useGetUserGaugePositions.tsx Outdated
Comment thread src/queries/useGetUserGaugePositions.tsx
Comment thread package.json
@petrovska-petro
Copy link
Copy Markdown
Collaborator Author

and probably also somewhat out of scope, but i noted that even if connected to opti goerli the same gauges show up. so somewhere we should start becoming chain aware. (a subgraph will always be for a single specific chain only, for a different chain a different subgraph will be needed)

added the chain awareness in 9249a30

@petrovska-petro
Copy link
Copy Markdown
Collaborator Author

this does raise a new question; what if we want to enable harvests for both?

another item was planning to scope on the ui side is to allow multi-select on the options and to do a multi-call in the safe app to enable both atomically

Copy link
Copy Markdown
Contributor

@gosuto-inzasheru gosuto-inzasheru left a comment

Choose a reason for hiding this comment

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

i need to refresh after switching chains, in order to refresh the gauge dropdown

maybe open a separate issue for this?

@gosuto-inzasheru
Copy link
Copy Markdown
Contributor

because the rest lgtm and can be merged imo :)

@petrovska-petro petrovska-petro merged commit 2858b19 into feat/safe-app-fix Aug 24, 2023
@petrovska-petro petrovska-petro deleted the feat/subgraph-query-ui-test branch August 24, 2023 09:47
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.

2 participants