Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Use shadcn components#154

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
mayur1377 wants to merge1 commit intoquicksnip-dev:main
base:main
Choose a base branch
Loading
frommayur1377:add-shadcn

Conversation

@mayur1377
Copy link
Contributor

Description

add shad-cn components

Type of Change

  • 🛠 Improvement to an existing snippet

note : there are some linting errors from the already pre-built shad-cn components , would recommend to disable folder that folder

  { ignores: ["node_modules", "dist", "build", "src/component/ui"] },

@netlify
Copy link

netlifybot commentedJan 3, 2025
edited
Loading

Deploy Preview forquicksnip ready!

NameLink
🔨 Latest commitea49c91
🔍 Latest deploy loghttps://app.netlify.com/sites/quicksnip/deploys/677808e6dd9f3b00083b31e8
😎 Deploy Previewhttps://deploy-preview-154--quicksnip.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 yourNetlify site configuration.

@mayur1377mayur1377 changed the titleAdd shadcnUse shadcn componentsJan 3, 2025
@psychlone77
Copy link
Collaborator

Instead of ignoring thecomponents/ui folder, we could runnpm run lint -- --fix to fix those issues. Since those files are also technically part of the codebase which could be modified in the future, better to have those files also kept properly.

Mathys-Gasnier reacted with thumbs up emoji

@mayur1377
Copy link
ContributorAuthor

Instead of ignoring thecomponents/ui folder, we could runnpm run lint -- --fix to fix those issues. Since those files are also technically part of the codebase which could be modified in the future, better to have those files also kept properly.

agree , but it would literally just be an overkill , to keep running it each and every time to fix the linting errors for the already pre built components
i would recommend two option here ,
-> ignore the components/ui folder which has all the pre-built shad-cn components which might hardly require some changes

-> just add the npm run lint fix command to the workflow file

Screenshot 2025-01-03 at 9 17 36 PM

@Mathys-Gasnier
Copy link
Collaborator

I'm too thinking we should ignore the components from shad-cn, it will just be a pain to reformat them each time.

mayur1377 reacted with thumbs up emoji

@psychlone77
Copy link
Collaborator

agree , but it would literally just be an overkill , to keep running it each and every time to fix the linting errors for the already pre built components.
i would recommend two option here ,
-> ignore the components/ui folder which has all the pre-built shad-cn components which might hardly require some changes
-> just add the npm run lint fix command to the workflow file

We will only have to fix the linting errors once when someone adds a new 'shadcn component' to the project. Since 'shadcn' components is still someone else' s code and we are technically just copy pasting the components into the project, we should still make sure that code follows the project's eslint rules.

Runningnpm run lint -- --fix is an option, but might introduce changes which the person who made the commit might not have expected. It's better for the build to fail and show where error was, instead of it automatically making changes.

Mathys-Gasnier, mayur1377, and jjcantu reacted with thumbs up emoji

@mayur1377
Copy link
ContributorAuthor

mayur1377 commentedJan 7, 2025
edited
Loading

any updates of still using shadcn?

@Mathys-Gasnier
Copy link
Collaborator

Fix conflicts and errors of your PR before we review again

@jjcantu
Copy link
Contributor

@Mathys-Gasnier I think we should probably open up another MR for this. It is blocking a lot of other developers from contributing.

@psychlone77psychlone77 added futureMight implement in the future update neededCode needs to be updated. labelsJan 17, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@psychlone77psychlone77Awaiting requested review from psychlone77

@saminjaysaminjayAwaiting requested review from saminjay

@Mathys-GasnierMathys-GasnierAwaiting requested review from Mathys-Gasnier

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

futureMight implement in the futureupdate neededCode needs to be updated.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mayur1377@psychlone77@Mathys-Gasnier@jjcantu

[8]ページ先頭

©2009-2025 Movatter.jp