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

Updated Create Pool Flow with Presets #246

Open
wants to merge 62 commits into
base: v2
Choose a base branch
from

Conversation

thevolcanomanishere
Copy link
Collaborator

@thevolcanomanishere thevolcanomanishere commented Dec 9, 2024

This has the updated Amplification Parameter preset flow for stable pools, updated progression and summary sidebars.

What isn't in this PR is an in-page tx (it still uses a DynamicPreview modal), we'll do that in BFE-410:

@PaulMcInnis PaulMcInnis self-assigned this Dec 11, 2024
Copy link

vercel bot commented Dec 11, 2024

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

Name Status Preview Comments Updated (UTC)
storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 23, 2024 11:55pm
v2-honey ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 23, 2024 11:55pm
v2-hub ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 23, 2024 11:55pm
9 Skipped Deployments
Name Status Preview Comments Updated (UTC)
bartio-dex ⬜️ Ignored (Inspect) Visit Preview Dec 23, 2024 11:55pm
bartio-honey ⬜️ Ignored (Inspect) Visit Preview Dec 23, 2024 11:55pm
bartio-lend ⬜️ Ignored (Inspect) Visit Preview Dec 23, 2024 11:55pm
bartio-perps ⬜️ Ignored (Inspect) Visit Preview Dec 23, 2024 11:55pm
bartio-station ⬜️ Ignored (Inspect) Visit Preview Dec 23, 2024 11:55pm
berajs-docs ⬜️ Ignored (Inspect) Dec 23, 2024 11:55pm
v2-bend ⬜️ Ignored (Inspect) Dec 23, 2024 11:55pm
v2-berps ⬜️ Ignored (Inspect) Dec 23, 2024 11:55pm
v2-dex ⬜️ Ignored (Inspect) Visit Preview Dec 23, 2024 11:55pm

…meters and using the existing Dynamic Preview to actually create the pool and do approvals.
Copy link
Contributor

@bearpong bearpong left a comment

Choose a reason for hiding this comment

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

Please, clean it up a bit and review all copy and titles with the one on figma. I think they changed but no one told you. You can double check

apps/hub/src/app/pools/components/oracle-input.tsx Outdated Show resolved Hide resolved
Comment on lines 85 to 88
function showStep(previewStep: number) {
// NOTE: if switched to using an ENUM / Map for steps we could do something more intelligent here.
return completedSteps.includes(previewStep);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please let's switch to enum, this index usage is a lil messy

apps/hub/src/app/pools/create/CreatePageContent.tsx Outdated Show resolved Hide resolved
});

const getStepVerification = (): {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move this into another file pls

}
};

// import { gql } from "@apollo/client";
Copy link
Contributor

Choose a reason for hiding this comment

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

we can't have references to the chain name in the codebase, please use the variable.

Can you pls share the error in dm? I'd like to help u, we can't merge the pr with hardcoded query

return {
...swrResponse,
refresh: () => void swrResponse.mutate(),
// FIXME we should probably have a polling rate so this expires?
Copy link
Contributor

Choose a reason for hiding this comment

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

you might even use useSWRImmutable, whitelisting can only change with a governance proposal

return {
...swrResponse,
refresh: () => void swrResponse.mutate(),
// FIXME we should probably have a polling rate so this expires?
Copy link
Contributor

Choose a reason for hiding this comment

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

for sure i wouldnt poll it on interval, and you might disable revalidateOnFocus too

Comment on lines 155 to 161
createButton: "#e6b434",
processStepBackground: "#373332",
highlight: "#0284C7",
semanticSuccessForeground: "#4ADE80",
semanticSuccessBackground: "#141D10",
semanticBlueSuccessForeground: "#38BCF8",
semanticBlueSuccessBackground: "#121A1C",
Copy link
Contributor

Choose a reason for hiding this comment

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

we cannot have new ds styles in this PR. please use legacy styles

Copy link
Collaborator

@PaulMcInnis PaulMcInnis Dec 23, 2024

Choose a reason for hiding this comment

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

responding here since I cant reply to the other comments:

  • RE the polling, I think we probably need to since this is specifically price data we are fetching (I think maybe you confused this SWR with the one in the whitelisted BGT PR 👀 ), so I've set it to SLOW (3mins)

Copy link
Collaborator

Choose a reason for hiding this comment

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

RE: ds styles, reverted back to green styling like in existing code 👍

…rect fetch from the codebase + Poll for bex api prices on a slow rate
{titles.map((title, index) => (
<div
key={index}
title={(isStepSelectable(index) && verifiedSteps.errors[index]) || ""}
className={cn(
"relative",
"relative w-full",
Copy link
Collaborator

@PaulMcInnis PaulMcInnis Dec 23, 2024

Choose a reason for hiding this comment

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

maybe worth running w-full by bobby/neph @bearpong ?

we had originally agreed to tile the steps so they might be surprised by w-full on each card

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.

None yet

3 participants