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.

Sign up for GitHub

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

Jump to bottom

fix: Collection page query over-fetching #11039

Merged
merged 11 commits into from
Oct 3, 2024
Merged

fix: Collection page query over-fetching #11039

merged 11 commits into from
Oct 3, 2024

Conversation

hassnian
Copy link
Contributor

@hassnian hassnian commented Sep 24, 2024 •
edited
Loading

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

@netlify Netlify
Copy link

netlify bot commented Sep 24, 2024 •
edited
Loading

✅ 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 •
edited
Loading

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 •
edited
Loading

@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 •
edited
Loading

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

Jarsen136
Jarsen136 approved these changes Sep 29, 2024
View reviewed changes
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

@hassnian hassnian mentioned this pull request Sep 30, 2024
Closed
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Oct 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@hassnian
Copy link
Contributor Author

hassnian commented Oct 2, 2024 •
edited
Loading

@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
preschian
preschian approved these changes Oct 3, 2024
View reviewed changes
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

components/collection/CollectionHeader.vue
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

@vikiival vikiival merged commit 6565469 into kodadot:main Oct 3, 2024
22 checks passed
@vikiival vikiival mentioned this pull request Oct 7, 2024
Merged
@preschian preschian mentioned this pull request Oct 7, 2024
Closed
4 tasks
This was referenced Oct 7, 2024
Merged
Merged
Merged
This was referenced Oct 15, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@preschian preschian preschian approved these changes

@Jarsen136 Jarsen136 Jarsen136 approved these changes

Assignees
No one assigned
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
Milestone
No milestone
4 participants
@hassnian @Jarsen136 @preschian @vikiival