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

#6708 - how to use images in code #6731

Merged
merged 2 commits into from
Aug 17, 2023

Conversation

thnaylor
Copy link
Contributor

@thnaylor thnaylor commented Aug 16, 2023

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

  • Bugfix
  • Feature
  • Refactoring

Context

Did your issue had any of the "$" label on it?

Screenshot

Copilot Summary

🤖 Generated by Copilot at 50d178b

This pull request enhances the style guide for image handling and adds a linting rule to enforce it. It aims to improve the performance and accessibility of the NFT gallery website by using HTML tags and URL paths for images instead of JavaScript imports.

🤖 Generated by Copilot at 50d178b

ESLint warns us
Images in JavaScript
Harm the web in spring

@thnaylor thnaylor requested a review from a team as a code owner August 16, 2023 07:30
@thnaylor thnaylor requested review from preschian and daiagi and removed request for a team August 16, 2023 07:30
@netlify
Copy link

netlify bot commented Aug 16, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit c76b01a
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64dd0ed116e8f700082bf23d
😎 Deploy Preview https://deploy-preview-6731--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.

@reviewpad
Copy link
Contributor

reviewpad bot commented Aug 16, 2023

AI-Generated Summary: This pull request addresses issue #6708, regarding the use of images in the code. It introduces changes to both the .eslintrc.js and STYLE_GUIDE.md files. In .eslintrc.js, imports have been restricted and rules updated to include components in various formats (JS, TS, VUE). Additionally, the 'no-restricted-imports' rule has been added, with a warning message advising against directly importing image files using JavaScript. Instead, the use of HTML tags and URL paths is recommended.

This change has also been reflected in the STYLE_GUIDE.md, which now includes a new section on how to work with images, mandates kebab-case for image file naming and recommends the .webp format. Example codes for bad and good practice were added as well. As such, this pull request aims to enforce better practices for using images in code, enhancing readability and maintainability.

@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels Aug 16, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented Aug 16, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@yangwao yangwao requested a review from vikiival August 16, 2023 12:46
@yangwao
Copy link
Member

yangwao commented Aug 16, 2023

exactly

image

.eslintrc.js Outdated
],
rules: {
'no-restricted-imports': [
'warn',
Copy link
Member

Choose a reason for hiding this comment

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

Cool. Can we force it as an 'error'? Haha 🤣

Copy link
Contributor Author

@thnaylor thnaylor Aug 16, 2023

Choose a reason for hiding this comment

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

Thanks, I have updated the rule to an 'error'.

The linter will now  'ELIFECYCLE  Command failed with exit code 1" if an image import is found.

files: [
'layouts/**/*.{js,ts,vue}',
'pages/**/*.vue',
'components/**/*.{js,ts,vue}',
Copy link
Member

Choose a reason for hiding this comment

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

oh, need more folders, just in case

utils, stores, services, plugins, composables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The following suggested directories have been added.

@codeclimate
Copy link

codeclimate bot commented Aug 16, 2023

Code Climate has analyzed commit c76b01a and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Aug 16, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@yangwao
Copy link
Member

yangwao commented Aug 17, 2023

pay 50 usd

@yangwao yangwao merged commit 929a427 into kodadot:main Aug 17, 2023
13 of 14 checks passed
@yangwao
Copy link
Member

yangwao commented Aug 17, 2023

😍 Perfect, I’ve sent the payout
💵 $50 @ 4.75 USD/DOT ~ 10.526 $DOT
🧗 123PCJXhjb15i6JwVVRGd7KvN2sSNZrtPjr5doJq9oWctoTt
🔗 0x8483e01ccbf236e7ca4b27308fa3e4fcfe4603d45cbaf76e19d43950cfbb28a2

🪅 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 Aug 17, 2023
@vikiival vikiival linked an issue Aug 17, 2023 that may be closed by this pull request
This was referenced Aug 18, 2023
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 small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Style guide: how to use images in the code
3 participants