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

add contributing and first_time pages #2644

Merged
merged 4 commits into from
Mar 24, 2022

Conversation

preschian
Copy link
Member

@preschian preschian commented Mar 23, 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.

Screen Shot 2022-03-23 at 2 34 21 PM

Screen Shot 2022-03-23 at 2 34 31 PM

@netlify
Copy link

netlify bot commented Mar 23, 2022

Deploy Preview for koda-nuxt ready!

Name Link
🔨 Latest commit 0a0c690
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/623c58939415b7000af00874
😎 Deploy Preview https://deploy-preview-2644--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.

@preschian preschian marked this pull request as ready for review March 23, 2022 08:55
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

Hey, we use MarkdownIt package with i18n plugin why would you remove it?
Plus do we really need this highlight.js stuff? I think it could be done using bulma component.

@preschian
Copy link
Member Author

preschian commented Mar 23, 2022

Hey, we use MarkdownIt package with i18n plugin why would you remove it? Plus do we really need this highlight.js stuff? I think it could be done using bulma component.

maybe I miss something. I didn't mean to remove the existing one, can you refer to which file? in package.json I only added highlight.js and vue-md-loader

highlight.js is for syntax highlighting, especially in FIRST_TIME.md

without with highligh.js
Screen Shot 2022-03-23 at 4 38 00 PM Screen Shot 2022-03-23 at 4 38 37 PM

I couldn't find bulma component for syntax highlight. can you send me the direct link, I only find this github issue jgthms/bulma#690

@roiLeo
Copy link
Contributor

roiLeo commented Mar 23, 2022

We use in some component vue-markdown-render to render md content (take a look at DescriptionWrapper.vue, GalleryItem.vue ...)

maybe I miss something. I didn't mean to remove the existing one, can you refer to which file? in package.json I only added highlight.js and vue-md-loader

Oups, my eyes saw a deleted line (markdown-it) in the yarn.lock and I thought the package was gone.

highlight.js is for syntax highlighting, especially in FIRST_TIME.md

Rather than integrating vue-md-loader could you try to use nuxtjs/markdownit-loader as it seems to have highlight.js features.

I couldn't find bulma component for syntax highlight. can you send me the direct link, I only find this github issue jgthms/bulma#690

My bad, I didn't understand that it was a code block

@preschian
Copy link
Member Author

preschian commented Mar 23, 2022

I tried https://github.com/nuxt-contrib/markdownit-loader before, but got an error when consuming the .md files. in the first commit, I end up with @nuxtjs/markdownit. working well, but there is a warning message when doing local development.
Screen Shot 2022-03-23 at 5 25 56 PM

pipeline for the snyk scanner also fail, I assume because of that package (I can't see snyk log) https://app.snyk.io/org/kodadot/pr-checks/474ef4f3-3452-48a5-905c-e24fd2fd1fb6

those packages actually have different use cases:

  • vue-markdown-render is rendered vue component from string of markdown
  • vue-md-loader is for direct consuming .md files, and convert it into vue component

another solution is to find another loader to turn .md files into a string, and then consume it with vue-markdown-render. tried this one, but fail and seems old https://github.com/unindented/markdown-it-loader

let me try another loader

@preschian
Copy link
Member Author

@roiLeo updated, following your suggestions, now use vue-markdown-render instead

Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

Final note on small things I would change:

  • remove ContentMarkdown component and keep the logic in pages
  • change snake_case page to kebab-case
  • create an utils file with highlight.js config

@yangwao
Copy link
Member

yangwao commented Mar 24, 2022

@preschian
Copy link
Member Author

in #2021: it seems like limitation with @nuxt/content only can consume markdown from the content folder

In this PR, extend webpack config using raw-loader. It can import any files from javascript and convert them into a string. and then render the markdown content with the existing component vue-markdown-render.

@yangwao
Copy link
Member

yangwao commented Mar 24, 2022

pay 100 usd

@yangwao yangwao merged commit 9177efb into kodadot:main Mar 24, 2022
@yangwao
Copy link
Member

yangwao commented Mar 24, 2022

😍 Perfect, I’ve sent the payout
💵 $100 @ 159.3 USD/KSM ~ 0.628 $KSM
🧗 DY4SQF2iD456tH89aQtz5wv1EV3BbSW8wKKuMcwbmXaj1pM
🔗 0x453baad07c7157dfcdb06315bf5bc9e3532ea6fb7ee3267031d526708c1570e9

🪅 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 Mar 24, 2022
@yangwao
Copy link
Member

yangwao commented Mar 24, 2022

oh I forgot, if we can link contribute to footer 😇

@preschian preschian mentioned this pull request Mar 24, 2022
17 tasks
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.

Embed Contributing.md & First_time.md to kodadot web
3 participants