-
-
Notifications
You must be signed in to change notification settings - Fork 361
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
fix: quick buy infinite loading #9484
Conversation
SUCCESS @pbkompasz PR for issue #9477 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime! |
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Test is failing due to |
@@ -5,6 +5,7 @@ | |||
scroll="clip" | |||
class="z-[1000]" | |||
content-class="modal-width" | |||
data-testid="confirm-modal" |
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.
for some reason i could not find this data-testid on the generated page
await page.goto(ITEMS_PATH) | ||
|
||
await test.step('Buy now from explorer', async () => { | ||
await page.getByTestId('item-buy').first().click() |
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.
you have to hover over the first card for the buy now button to appear
|
||
await test.step('Buy now from explorer', async () => { | ||
await page.getByTestId('item-buy').first().click() | ||
await expect(await page.getByTestId('confirm-modal')).toBeVisible() |
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 may have to add timeout, since its not always fast
nope, its actually the test itself, lacking some lines, appreciate it tho regarding solution, buy now no longer on infinite loading ✔️ |
@pbkompasz this section it's working at it should, so no changes needed there the issue is that , it's failing to connect to the websocket when trying to subscribe for price changes , @kodadot/internal-dev can you guys check that ? handle the onError case inside thanks |
Cc @preschian |
oooh, this one, when trying to target subscriptions, should use old endpoints. @hassnian @pbkompasz could you check if this file is working or not https://github.com/kodadot/nft-gallery/blob/main/utils/websocket.ts#L4-L8 recently I changed that files |
checking~ |
@hassnian @pbkompasz Sorry there was a typo, fixed here: #9491 |
Quality Gate passedIssues Measures |
Code Climate has analyzed commit 51c3d2a and detected 0 issues on this pull request. View more on Code Climate. |
const loading = computed(() => !autoteleport.value?.isReady || props.loading) | ||
const loading = ref(props.loading) | ||
|
||
watch( | ||
() => autoteleport.value?.isReady, | ||
(val) => { | ||
loading.value = !val | ||
}, | ||
) |
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.
How about
const loading = computed(() => !autoteleport.value?.isReady || props.loading) | |
const loading = ref(props.loading) | |
watch( | |
() => autoteleport.value?.isReady, | |
(val) => { | |
loading.value = !val | |
}, | |
) | |
const loading = computed(() => autoteleport.value?.isReady ? !autoteleport.value.isReady : props.loading) |
const loading = ref(props.loading) | ||
|
||
watch( | ||
() => autoteleport.value?.isReady, | ||
(val) => { | ||
loading.value = !val | ||
}, | ||
) |
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.
@hassnian close this one then? |
Sure, since it's already working If anything as I said the only thing would be to handle the case where the websocket connection fails |
Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.
👇 __ Let's make a quick check before the contribution.
PR Type
Context
Before submitting pull request, please make sure:
Optional
Did your issue had any of the "$" label on it?
Community participation
Screenshot 📸
Copilot Summary