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

feat(2195): mini-pulse-history component #2884

Merged
merged 23 commits into from
May 6, 2022
Merged

Conversation

KngZhi
Copy link
Contributor

@KngZhi KngZhi commented Apr 19, 2022

Thank you for your contribution to the KodaDot NFT gallery.
👇 _ Let's make a quick check before the contribution.

PR type

  • Bugfix
  • Feature
  • Refactoring

What's new?

Before submitting Pull Request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Optional

  • I've tested it at </rmrk/collection/26902bc2f7c20c546a-1FVG7>
  • I've tested PR on mobile and everything seems works
  • I found edge cases
  • I've written some unit tests 🧪

Had issue bounty label?

  • Fill up your KSM address: Payout

Community participation

Screenshot

  • My fix has changed something on UI; a screenshot is best to understand changes for others.

image

@KngZhi KngZhi requested a review from a team as a code owner April 19, 2022 05:33
@KngZhi KngZhi requested review from prachi00 and removed request for a team April 19, 2022 05:33
@kodabot
Copy link
Collaborator

kodabot commented Apr 19, 2022

WARNING @KngZhi PR for issue #2195 which isn't assigned to you. Please be warned that this PR may get rejected if there's another assignee for issue #2195

@netlify
Copy link

netlify bot commented Apr 19, 2022

Deploy Preview for koda-nuxt ready!

Name Link
🔨 Latest commit 5100a2d
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/62746b9454e0390008314f4a
😎 Deploy Preview https://deploy-preview-2884--koda-nuxt.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 settings.

@KngZhi
Copy link
Contributor Author

KngZhi commented Apr 19, 2022

Hey @yangwao this is a demo page, do you prefer this UI for desktop and mobile.

Besides, I think we need to update the resolver to retrieve relevant data, but I'm kinda stuck with that, is there anyone I can resort to or communicate with.

@yangwao
Copy link
Member

yangwao commented Apr 23, 2022

nice!

image

@KngZhi KngZhi changed the title WIP: mini-pulse-history component [DONE] mini-pulse-history component Apr 25, 2022
@KngZhi
Copy link
Contributor Author

KngZhi commented Apr 25, 2022

Brief Summary:

feat:

  1. a mini-history component for the series and spotlight(EChart)
  2. queries for historical data, and PLEASE correct me if there had any more efficient or clean way to do GraphQL. 😄 [1]

fix:

  1. using computed data for the spotlight
  2. [spotlight] fix pageSize problem
  3. align-center for each column

refactor:

  1. using computed data as the datasource for the spotlight

Following TODOs(for other PRs):

  1. Since the EChart supports SVG format, which is more friendly to Mobile, I want to refactor other pages that use Chart.js
  2. Clean useless code inside spotlight and series, besides, refactor some duplicate code.

@yangwao yangwao requested a review from vikiival April 25, 2022 10:15
Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

This should be done via rubick resolver

components/series/SeriesTable.vue Outdated Show resolved Hide resolved
components/series/utils.ts Show resolved Hide resolved
components/series/utils.ts Outdated Show resolved Hide resolved
components/series/utils.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
components/spotlight/SpotlightTable.vue Outdated Show resolved Hide resolved
components/spotlight/SpotlightTable.vue Outdated Show resolved Hide resolved
components/spotlight/SpotlightTable.vue Outdated Show resolved Hide resolved
components/spotlight/SpotlightTable.vue Outdated Show resolved Hide resolved
@vikiival
Copy link
Member

Is it possible to specify the data you need?

I would rather write resolver for this kodadot/rubick#58

@KngZhi
Copy link
Contributor Author

KngZhi commented Apr 26, 2022

The data structure would be like this

{
  "Collection_id": {
    "2021-02-04": 2,
    "2022-02-05": 3
  }
}

The key is date that buy event happened and the value is how many times buy transactions happened.

I'm outside now, using phone, sorry with the format.

@KngZhi
Copy link
Contributor Author

KngZhi commented Apr 26, 2022

Besides, could you check kodadot/rubick#54

@yangwao
Copy link
Member

yangwao commented May 5, 2022

pay 150 usd

@yangwao
Copy link
Member

yangwao commented May 5, 2022

Wait, some graphs seem suspicious same, why is it like that?

Screen.Recording.2022-05-05.at.14.37.35.mov

@yangwao
Copy link
Member

yangwao commented May 5, 2022

Looks like overlay bug

image

@KngZhi
Copy link
Contributor Author

KngZhi commented May 5, 2022

Wait, some graphs seem suspicious same, why is it like that?

Screen.Recording.2022-05-05.at.14.37.35.mov

hmm, seems some data duplicated. Will fix it right now.

@KngZhi
Copy link
Contributor Author

KngZhi commented May 6, 2022

@yangwao
Copy link
Member

yangwao commented May 6, 2022

@KngZhi you can directly upload videos to github input box to save you time like on this video :)

Screen.Recording.2022-05-06.at.09.37.25.mov

@yangwao
Copy link
Member

yangwao commented May 6, 2022

pay 150 usd

1 similar comment
@yangwao
Copy link
Member

yangwao commented May 6, 2022

pay 150 usd

@yangwao
Copy link
Member

yangwao commented May 6, 2022

Error completing request

lol what payout bot 👀

@yangwao
Copy link
Member

yangwao commented May 6, 2022

pay 100 usd

@KngZhi
Copy link
Contributor Author

KngZhi commented May 6, 2022

@KngZhi you can directly upload videos to github input box to save you time like on this video :)

Screen.Recording.2022-05-06.at.09.37.25.mov

@yangwao oh, my, thanks, learned a new technique, hahaha 😄

Screen.Recording.2022-05-06.at.3.41.52.PM.mov

@yangwao
Copy link
Member

yangwao commented May 6, 2022

pay 100 usd

@yangwao
Copy link
Member

yangwao commented May 6, 2022

oh got it, your address has 47 chars when I matching only 46 hmm, got it

@KngZhi
Copy link
Contributor Author

KngZhi commented May 6, 2022

Okay.

@yangwao
Copy link
Member

yangwao commented May 6, 2022

hey I've put in que and mentioned in #2941 so seems you have a different address than I wrote matching regexp.

Also, @KngZhi invite you for bounty help to figure out the right matching stuff for the KSM address than I'm using one in #2941 :)

I'll merge it so it's out there and can continue with resolver and retroactively repay, don't worry 🤞

@yangwao yangwao enabled auto-merge May 6, 2022 07:53
@yangwao yangwao disabled auto-merge May 6, 2022 07:54
@yangwao yangwao merged commit 5835fdb into kodadot:main May 6, 2022
@vikiival
Copy link
Member

vikiival commented May 6, 2022

🚀

@yangwao
Copy link
Member

yangwao commented May 6, 2022

pay 100 usd

1 similar comment
@yangwao
Copy link
Member

yangwao commented May 6, 2022

pay 100 usd

@yangwao
Copy link
Member

yangwao commented May 6, 2022

😍 Perfect, I’ve sent the payout
💵 $100 @ 123.95 USD/KSM ~ 0.807 $KSM
🧗 DaWoVNNhBTdqJ5iPN3UcaLeamkPpJeGWdrs8LhvX9TQbadc
🔗 0x52329283d8a8ae1d8806f937f74f5260db1a9f3de2f23c94b82a90b1965db7da

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label May 6, 2022
@KngZhi KngZhi deleted the issue-2195 branch May 6, 2022 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paid pull-request has been paid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants