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
app: Dynamically create order page match cards #1864
Conversation
Updates the order page to dynamically create match cards instead of creating them with template. This allows the user to see new matches without having to reload the page.
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 work! A couple of observations from a cursory scan.
page.matchCardTmpl.removeAttribute('id') | ||
page.matchCardTmpl.remove() |
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.
Can also Doc.cleanTemplates
.
this.start().then( | ||
() => { | ||
this.showMatchCards() | ||
} | ||
) |
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.
Why not just put this.showMatchCards()
at the end of start
?
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.
True
Doc.tmplElement(matchCard, 'matchID').textContent = match.matchID | ||
|
||
const time = new Date(match.stamp) | ||
Doc.tmplElement(matchCard, 'matchTime').textContent = time.toLocaleTimeString('en-GB', { | ||
year: 'numeric', | ||
month: 'short', | ||
day: 'numeric' |
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.
const tmpl = Doc.parseTemplate(matchCard)
tmpl.matchID.textContent = ...
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.
Looks good.
The accelerate button does not seem to be showing up for the orders page most of the time. Do you also see, or not see, this? I can make an issue or you can if so. Unless its an easy fix. I can't remember, but do the accelerations handle accelerations for different matches for the same order? I don't remember being able to choose. You could have a booked order match half and start settling then match with another person and you need to accelerate both? I guess the second match uses the change from the first, so can only accelerate in order? |
Acceleration is at the level of the order, not the match, because they are all in a chain, spending change to fund subsequent matches, as you noted. |
client/core/helpers.go
Outdated
return readers | ||
} | ||
|
||
// IsMaker returns if the user is the maker in this match. |
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.
Looks to me like the rest of matchReader
can go now. There are no invocations of (*OrderReader).MatchReaders
remaining, and the remaining methods aren't used (some share names with OrderReader
methods, but the matchReader
variants aren't used.)
const showMakerSwap = !m.isCancel && (makerSwapCoin(m) || !m.revoked) | ||
const showTakerSwap = !m.isCancel && (takerSwapCoin(m) || !m.revoked) | ||
const showMakerRedeem = !m.isCancel && (makerRedeemCoin(m) || !m.revoked) | ||
const showTakerRedeem = !m.isCancel && (m.side !== OrderUtil.Maker) && (takerRedeemCoin(m) || !m.revoked) | ||
const showRefund = !m.isCancel && (m.refund || (m.revoked && m.active)) | ||
|
||
if (showMakerSwap) Doc.show(tmpl.makerSwap) | ||
else Doc.hide(tmpl.makerSwap) | ||
if (showTakerSwap) Doc.show(tmpl.takerSwap) | ||
else Doc.hide(tmpl.takerSwap) | ||
if (showMakerRedeem) Doc.show(tmpl.makerRedeem) | ||
else Doc.hide(tmpl.makerRedeem) | ||
if (showTakerRedeem) Doc.show(tmpl.takerRedeem) | ||
else Doc.hide(tmpl.takerRedeem) | ||
if (showRefund) Doc.show(tmpl.refund) | ||
else Doc.hide(tmpl.refund) |
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.
Some of the existing code here is clearly from very early development. There is also Doc.setVis
now, so you could do 1 or 2 lines per element, e.g.
Doc.setVis(!m.isCancel && (makerSwapCoin(m) || !m.revoked), tmpl.makerSwap)
cff311f
to
a430b7b
Compare
Updates the order page to dynamically create match cards instead of creating them with the template. This allows the user to see new matches without having to reload the page.