-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat: connect Expeditions modal to the expeditions api #1433
base: develop
Are you sure you want to change the base?
feat: connect Expeditions modal to the expeditions api #1433
Conversation
fix(expeditions-context): add missing setRewards
…button loading feature(expeditions card): add claimed styles chore(expeditions): fix typo feature(expeditions): extract logic for button text
❌ Deploy Preview for swapr failed.
|
GENERATE_SOURCEMAP=false | ||
REACT_APP_SWAPR_API_BASE_URL=http://localhost:4000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might mess up Netlify deployment tho.
…-the-expeditions-api
Doing some testing, here are some findings and suggested changes. All of the below were tested on Gnosis Chain:
Depositing liq: https://gnosisscan.io/tx/0x84a9483570a1a028be907252242dad386e80c532563a06acbf64ef47853a05a4
Besides the above, functionally this seems sound. Great stuff! The next step is |
So for each completed task user would see a timer on the button showing the same countdown. Maybe instead, we can show the number of current week + single timer until week ends? For example it would be below 'Learn more' sign It would also suggest when new tasks will be available.
Something like this? Screencast.from.15-09-22.01.32.40.webm |
My interpretation is that showing the time until the specific action is available again scales better, since we have been discussing a daily campaign, event-specific campaigns, etc. I defer to you folks on the team, but I would personally be for a way of indicating the time until the next claim directly in or around that task.
This is exactly what I had envisioned! Any thoughts from your end? Chatted with @zamli about how this could be visualized so defer a decision to him. |
Fair enough, I had only weekly challenges in mind and forgot about daily and other possible tasks.
Personally I love it, it could stay like this all the time 😎 |
…-the-expeditions-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, good work Adam. Just left some comments
@@ -16,11 +16,14 @@ | |||
"cypress:run": "cross-env cypress run -r mochawesome -s 'tests/cypress/integration/smoke/*'", | |||
"integration-test": "start-server-and-test 'serve build -l 3000' http://localhost:3000 'cypress run'", | |||
"synpress:ct": "cross-env start-server-and-test 'react-app-rewired start' http-get://localhost:3000 'yarn synpress:run $TEST_PARAMS'", | |||
"codegen": "yarn codegen:socket && yarn codegen:expeditions && yarn codegen:graphql", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks organized
export function capitalize(string: string) { | ||
return string[0].toUpperCase() + string.slice(1) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be done with CSS only. text-transform: capitalize
does the trick
<Row> | ||
<TYPE.White fontSize="12px" lineHeight="150%" textAlign={'center'}> | ||
Expeditions are missions you can complete on Swapr to earn “Star Fragments”. These can be redeemed for | ||
special NFT's and various other rewards. <StyledExternalLink href="#">Learn More</StyledExternalLink> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't forget to set this link.
const claim = async () => { | ||
if (isClaimed || isClaiming) { | ||
return | ||
} | ||
|
||
setIsClaiming(true) | ||
try { | ||
const address = await provider.getSigner().getAddress() | ||
const signature = await provider.getSigner().signMessage(ClaimRequestTypeEnum.DailyVisit) | ||
await ExpeditionsAPI.postExpeditionsClaim({ | ||
body: { | ||
address, | ||
signature, | ||
type: ClaimRequestTypeEnum.DailyVisit, | ||
}, | ||
}) | ||
|
||
const { claimedFragments, tasks } = await ExpeditionsAPI.getExpeditionsProgress({ | ||
address, | ||
}) | ||
|
||
// Update local state | ||
setTasks(tasks) | ||
setClaimedFragments(claimedFragments) | ||
} catch (error) { | ||
console.error(error) | ||
} finally { | ||
setIsClaiming(false) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic seems very similar in all 3 components. Maybe we can set a hook with this logic that receives a different ClaimRequestTypeEnum
type
}} | ||
className={activeTab === ExpeditionsTabs.REWARDS ? 'active' : ''} | ||
> | ||
{<Badge badgeTheme={claimableRewards ? 'green' : 'orange'}>{claimableRewards}</Badge>} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are curly braces required here?
src/modules/expeditions/index.tsx
Outdated
@@ -0,0 +1 @@ | |||
export {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should export something here
if (isClaiming) { | ||
buttonText = 'Claiming...' | ||
} else if (isClaimed) { | ||
buttonText = 'Claimed' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we present the claimed fragments too?
) | ||
|
||
export const StatusTag = ({ status }: { status: StatusTags }) => { | ||
if (status === 'loading') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if is an overkill but maybe "loading" should be a constant.
const LOADING = "loading"
if (status === 'loading') { | |
if (status === LOADING) { |
duration?: 'Weekly' | 'Daily' | ||
title: string | ||
description: string | ||
buttonText: React.ReactNode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's odd that buttonText
is a ReactNode
, shouldn't it be a string?
import { ContentWrapper, ExpeditionsLogo } from './ExpeditionsModal.styled' | ||
import { ExpeditionsRewards } from './ExpeditionsRewards' | ||
import { ExpeditionsTasks } from './ExpeditionsTasks' | ||
export interface ExpeditionsModalProps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would add a new line here.
export interface ExpeditionsModalProps { | |
export interface ExpeditionsModalProps { |
const rewards: RewardCardProps[] = ['common', 'uncommon', 'rare', 'epic', 'legendary', 'mythic'].map( | ||
(rarity, index) => ({ | ||
buttonText: index === 0 ? 'Expired' : index === 1 ? 'Claimed' : 'Claim', | ||
description: `Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis tempor, ante nec consectetur vulputate, eros nisi placerat neque, ut volutpat diam tellus ut sem.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't forget to update this description.
export const ExpeditionsRewards = () => { | ||
return ( | ||
<> | ||
{rewards.map((props, index) => ( | ||
<RewardCard key={index} {...props} /> | ||
))} | ||
</> | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In an arrow function we can remove the explicit return
.
It's just a suggestion but I think it is cleaner.
export const ExpeditionsRewards = () => { | |
return ( | |
<> | |
{rewards.map((props, index) => ( | |
<RewardCard key={index} {...props} /> | |
))} | |
</> | |
) | |
} | |
export const ExpeditionsRewards = () => | |
<> | |
{rewards.map((props, index) => ( | |
<RewardCard key={index} {...props} /> | |
))} | |
</> |
* add daily swap task
fb69230
to
0c5053c
Compare
ff47650
to
b3b3815
Compare
91e4f23
to
af06da1
Compare
Summary
Connects the Expeditions modal to the API hosted at https://api.swapr.site/v1.0/documentation
To Test
Testing this feature requires on-chain transactions. In order to claim weekly fragments. User has to deposit or stake $50 or more worth of liquidity on any of the three chains Swapr core supports: Mainnet, Arbitrum and Gnosis Chain.