-
-
Notifications
You must be signed in to change notification settings - Fork 366
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
🔧 MyOffer & OffersUserTable #6732
Conversation
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
AI-Generated Summary: This pull request includes changes to two Vue files related to the offer feature. In Furthermore, a new block of code has been added to Overall, these changes focus on improving the display and readability of price values and the addition of expiration time calculation. |
Reviewpad Report
|
const currentBlock = ref(async () => { | ||
const { apiInstance } = useApi() | ||
const api = await apiInstance.value | ||
const block = await api.rpc.chain.getHeader() | ||
return block.number.toNumber() | ||
}) | ||
|
||
const calcExpirationTime = (expirationBlock: number): string => { | ||
if (currentBlock.value === 0) { | ||
return 'computing' | ||
} | ||
if (currentBlock.value > expirationBlock) { | ||
return 'expired' | ||
} | ||
return formatSecondsToDuration(calcSecondsToBlock(expirationBlock)) | ||
} |
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 help with that
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.
yes, since I'm starting to duplicate this part of code
@@ -34,9 +34,9 @@ | |||
<NeoTableColumn | |||
v-slot="props" | |||
field="formatPrice" | |||
:label="$t('myOffer.price')" | |||
:label="`${$t(`offer.price`)} (${chainSymbol})`" |
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.
Maybe add that as param of translation?
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.
meh idk, it's used without chainSymbol in other components
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.
Smol
@kodadot/code-review-guild any idea why I can't fetch For now I've hardcoded |
Changing this line of code could fix this issue. Please also check other places that use it. // composables/useChain.ts
const { symbol } = assets(tokenId.value) Change to // composables/useChain.ts
const symbol = computed(() => assets(tokenId.value).symbol) |
Thanks it does work! |
Code Climate has analyzed commit fd99131 and detected 0 issues on this pull request. View more on Code Climate. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@prury Notification pulse button is out of scope, the feature hasn't been implemented yet