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

Integrated tailwindcss, refactored colors and rewrote NeoButton component #8141

Merged
merged 13 commits into from
Nov 23, 2023

Conversation

shashkovdanil
Copy link
Contributor

@shashkovdanil shashkovdanil commented Nov 18, 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

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

Optional

  • I've tested it at </ksm/collection>
  • I've tested PR on mobile
  • I've written unit tests 🧪
  • I've found edge cases

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

  • Fill up your DOT address: Payout

Community participation

Screenshot 📸

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

Copilot Summary

🤖[deprecated] Generated by Copilot at c239608

This pull request integrates Tailwind CSS into the NFT gallery project, enabling the use of Tailwind classes and utilities for creating and optimizing CSS styles. It configures PostCSS plugins for the Nuxt application and the UI library, and updates the dependencies and the NeoButton component accordingly. It also deletes the unused NeoButton.scss file.

🤖[deprecated] Generated by Copilot at c239608

We unleash the power of tailwindcss
We optimize and transform our styles
We defy the browser's tyranny with autoprefixer
We create a stunning NFT gallery of fire

@shashkovdanil shashkovdanil requested a review from a team as a code owner November 18, 2023 01:20
@shashkovdanil shashkovdanil requested review from preschian and roiLeo and removed request for a team November 18, 2023 01:20
Copy link

netlify bot commented Nov 18, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 4ee7bfe
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/655f239d42d94500084de329
😎 Deploy Preview https://deploy-preview-8141--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.

@kodabot
Copy link
Collaborator

kodabot commented Nov 18, 2023

SUCCESS @shashkovdanil PR for issue #8113 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime!

@shashkovdanil
Copy link
Contributor Author

@preschian @roiLeo Hi guys, if you like this improvement I may continue to rewrite other components in a similar style. Also you can see how I redid the colors, with this approach no more _theme.scss is needed

Copy link

socket-security bot commented Nov 18, 2023

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
tailwindcss 3.3.5 filesystem, environment +2 7.84 MB adamwathan

@shashkovdanil
Copy link
Contributor Author

@exezbcz I would appreciate it if you could help me test the button, I have looked at the buttons on different pages and everything seems to look good

@exezbcz
Copy link
Member

exezbcz commented Nov 18, 2023

@exezbcz I would appreciate it if you could help me test the button, I have looked at the buttons on different pages and everything seems to look good

Yes, what buttons should I test so far?

I will also fill the components with button designs soon

@shashkovdanil
Copy link
Contributor Author

@exezbcz yes, in this PR I've changed only buttons

@reviewpad reviewpad bot added large Pull request is large waiting-for-review labels Nov 18, 2023
@exezbcz
Copy link
Member

exezbcz commented Nov 18, 2023

image - text inside button on hover changing as a link - same with hover image

same here in cart sidebar
image

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 nice work but that's not what I was excepting, could you do your changes without affecting Kodadot app (only in libs/ui package). The package should be independent and should be used outside Kodadot.

please consider:

  • use TS over JS
  • use SCSS over CSS
  • add story preview for each component you create
  • :root colors not visible (on IDE) when using values instead of hex

nuxt.config.ts Outdated Show resolved Hide resolved
libs/ui/tailwind.config.js Outdated Show resolved Hide resolved
@preschian
Copy link
Member

@preschian @roiLeo Hi guys, if you like this improvement I may continue to rewrite other components in a similar style. Also you can see how I redid the colors, with this approach no more _theme.scss is needed

overall, I like it

@shashkovdanil
Copy link
Contributor Author

Hey @roiLeo I'll update PR

add story preview for each component you create

What do you mean? There is story NeoButton.story.vue, you want me to create other one?

:root colors not visible (on IDE) when using values instead of hex

I can use hex instead rgb, but then we lose the ability to change the alpha channel of colors (text-k-accent/50 -> translucent k-accent color). If we don't need this feature, I'll rewrite it in hex

@roiLeo
Copy link
Contributor

roiLeo commented Nov 20, 2023

What do you mean? There is story NeoButton.story.vue, you want me to create other one?

Did you try to run it? How does it look?

I can use hex instead rgb, but then we lose the ability to change the alpha channel of colors (text-k-accent/50 -> translucent k-accent color). If we don't need this feature, I'll rewrite it in hex

you still can set transparency with hex value, that was a suggestion for better visualization when looking at code.

@shashkovdanil
Copy link
Contributor Author

NeoButton.story.vue

Screenshot 2023-11-20 at 10 33 45

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.

<hr /> from wallet sidebar no longer seems to be visible

@shashkovdanil
Copy link
Contributor Author

shashkovdanil commented Nov 20, 2023

<hr /> from wallet sidebar no longer seems to be visible

Fixed, it was because of CSS reset from tailwind, I disabled it

libs/ui/postcss.config.js Outdated Show resolved Hide resolved
libs/ui/src/components/NeoButton/NeoButton.vue Outdated Show resolved Hide resolved
nuxt.config.ts Show resolved Hide resolved
@roiLeo
Copy link
Contributor

roiLeo commented Nov 21, 2023

Small conflicts here.

So what's next? are we going to gradually rewritte each components from libs/ui?

@shashkovdanil
Copy link
Contributor Author

So what's next? are we going to gradually rewritte each components from libs/ui?

If you like this approach, then yes I can rewrite other components

@shashkovdanil shashkovdanil requested a review from roiLeo November 21, 2023 14:51
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.

may someone double check?

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.

yeay, finally we can use tailwind. I hope didn't conflict with current styles

follow up issues:

@shashkovdanil
Copy link
Contributor Author

@preschian I'll create new tasks soon for other components. About nuxt module, we can use this if we plan to use tailwind not only in the UI kit, but also in the kodadot itself.

Could you merge this please, otherwise I'll have to fix conflicts in pnpm-lock frequently

Copy link

codeclimate bot commented Nov 23, 2023

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

View more on Code Climate.

Copy link

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
0.4% 0.4% Duplication

@preschian preschian added this pull request to the merge queue Nov 23, 2023
Merged via the queue into kodadot:main with commit 36aca68 Nov 23, 2023
17 checks passed
@preschian
Copy link
Member

Could you merge this please

cc @yangwao for the payout

@shashkovdanil
Copy link
Contributor Author

cc @yangwao friendly ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
large Pull request is large waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tailwindcss Integration
5 participants