-
Notifications
You must be signed in to change notification settings - Fork 7
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: allow exiting offers #62
Conversation
5446615
to
f64dd1b
Compare
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.
Will review again after the contract lands
wallet/src/service/Offers.ts
Outdated
@@ -206,45 +207,95 @@ export const getOfferService = ( | |||
return signSpendAction(offer.spendAction); | |||
}; | |||
|
|||
const cancelOffer = _id => { | |||
console.log('TODO: cancel offer'); | |||
const cancelOffer = async (id: number) => { |
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.
let's be consistent in terminology:
const cancelOffer = async (id: number) => { | |
const tryExitOffer = async (id: number) => { |
"cancel" suggests that it'll always work
wallet/src/components/Offer.tsx
Outdated
<div className="OfferOrigin" style={{ wordBreak: 'break-word' }}> | ||
<PetnameSpan name={sourceDescription} /> | ||
<i> via </i> | ||
<span className="Blue">{dappOrigin || origin}</span> | ||
{(dappOrigin || origin) && ( |
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.
consider const actualOrigin = dappOrigin || origin
so the condition doesn't need to be repeated on line 142
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.
Nice done.
wallet/src/service/Offers.ts
Outdated
pendingOffersNotifier, | ||
)) { | ||
console.log('pending offers', pendingOffers); | ||
pendingOffers?.forEach(([_, o]) => { |
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.
prefer control structures when available. for
loop allows continue
e.g.,
pendingOffers?.forEach(([_, o]) => { | |
if (!pendingOffers) continue; | |
for (const [_, o] of pendingOffers) { |
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.
Done
@@ -201,8 +190,8 @@ test('renders the controls', () => { | |||
expect(controls.find(Chip).at(1).text()).toContain('Decline'); | |||
}); | |||
|
|||
test.skip('renders the exit button while pending', () => { | |||
const pendingOffer = { ...offer }; | |||
test('renders the exit button while seated', () => { |
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.
glad to see these tests
fixes: #61
Tested making liquidation bids from CLI and exiting them from UI