Skip to content
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: Collection page query over-fetching #11039

Merged
merged 11 commits into from
Oct 3, 2024
Merged

Conversation

hassnian
Copy link
Contributor

@hassnian hassnian commented Sep 24, 2024

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Before

CleanShot 2024-09-25 at 11 59 01@2x

CleanShot 2024-09-25 at 11 52 19@2x

After

CleanShot 2024-09-25 at 11 53 59@2x

CleanShot 2024-09-25 at 11 59 25@2x

Screenshot 📸

  • My fix has changed something on UI;

Before

CleanShot 2024-09-25 at 12 06 01@2x

After

CleanShot 2024-09-25 at 12 01 13@2x

Copy link

netlify bot commented Sep 24, 2024

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 8a0cc9d
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/66fcf5e1aec24c00081a06a7
😎 Deploy Preview https://deploy-preview-11039--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hassnian hassnian marked this pull request as ready for review September 25, 2024 07:08
@hassnian hassnian requested a review from a team as a code owner September 25, 2024 07:08
@hassnian hassnian requested review from vikiival and Jarsen136 and removed request for a team September 25, 2024 07:08
@Jarsen136
Copy link
Contributor

Not sure if it's my network. It did not show the basic info about collection when I opened the link:
https://deploy-preview-11039--koda-canary.netlify.app/ahk/collection/u-2024
image

image

@hassnian
Copy link
Contributor Author

hassnian commented Sep 26, 2024

Not sure if it's my network. It did not show the basic info about collection when I opened the link:

https://deploy-preview-11039--koda-canary.netlify.app/ahk/collection/u-2024

image image

@Jarsen136 it's weird on local it's working, checking

@hassnian
Copy link
Contributor Author

hassnian commented Sep 28, 2024

@Jarsen136 fixed, can you check again

CleanShot 2024-09-28 at 09 52 46@2x

this page takes some time to load
CleanShot 2024-09-28 at 10 03 15@2x

@Jarsen136
Copy link
Contributor

Jarsen136 commented Sep 29, 2024

The chart can't be shown. It could be fixed on another issue as it's also happening in canary on my side.

image image

Copy link
Contributor

@Jarsen136 Jarsen136 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

Copy link

sonarcloud bot commented Oct 2, 2024

@hassnian
Copy link
Contributor Author

hassnian commented Oct 2, 2024

@Jarsen136 added #11055 in this pr -> 8a0cc9d

that page is heavy so the issue might be that it takes a lot of time ( related with #10416 ) , it's working on my end , can you try again ?

CleanShot 2024-10-02 at 12 22 17

@vikiival vikiival requested review from preschian and removed request for vikiival October 2, 2024 08:09
@Jarsen136
Copy link
Contributor

@Jarsen136 added #11055 in this pr -> 8a0cc9d

that page is heavy so the issue might be that it takes a lot of time ( related with #10416 ) , it's working on my end , can you try again ?

Awesome, it works well on my side. Thank you!
image

@Jarsen136 Jarsen136 added S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked labels Oct 2, 2024
Copy link
Member

@preschian preschian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

const route = useRoute()
const collectionId = computed(() => route.params.id.toString())

const { collection, refetch } = useCollectionMinimal({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collection doesnt have proper types before. would be good if we can fix that here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked
Projects
None yet
4 participants