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

Migrated card component to chakraUI #631

Merged
merged 12 commits into from
Jan 28, 2025
Merged

Conversation

Dishant1804
Copy link
Collaborator

Resolves #575

This PR migrates the card component to chakraUI, also the test cases for the same are modified

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

Works for me!

At what phase you think we'll be able to remove tailwindcss stuff?

@Dishant1804
Copy link
Collaborator Author

Works for me!

At what phase you think we'll be able to remove tailwindcss stuff?

If we are thinking to remove tailwind then lets go component by component and for every migration of component we will be able to achieve the end goal of removing tailwind, i can make changes in this PR if we really want to move away from tailwind

@arkid15r
Copy link
Collaborator

Works for me!
At what phase you think we'll be able to remove tailwindcss stuff?

If we are thinking to remove tailwind then lets go component by component and for every migration of component we will be able to achieve the end goal of removing tailwind, i can make changes in this PR if we really want to move away from tailwind

No, no action required. It was just a general question, no rush.

@Dishant1804
Copy link
Collaborator Author

Works for me!
At what phase you think we'll be able to remove tailwindcss stuff?

If we are thinking to remove tailwind then lets go component by component and for every migration of component we will be able to achieve the end goal of removing tailwind, i can make changes in this PR if we really want to move away from tailwind

No, no action required. It was just a general question, no rush.

okay sure then lets take baby steps towards the migration, lets first migrate the components, if we are sure that we can move out of tailwind completely (which is a bit difficult), we will do it then, till then we can migrate components step by step

@Dishant1804
Copy link
Collaborator Author

@arkid15r made the necessary changes, you can review it now :)

@Dishant1804 Dishant1804 requested a review from arkid15r January 28, 2025 19:16
@Rajgupta36
Copy link
Collaborator

Rajgupta36 commented Jan 28, 2025

Hi @Dishant1804, in this PR, we are using the Tooltip from Chakra instead of react-tooltip. Can we remove the react-tooltip dependency from package.json?

@Dishant1804
Copy link
Collaborator Author

Hi @Dishant1804, in this PR, we are using the Tooltip from Chakra instead of react-tooltip. Can we remove the react-tooltip dependency from package.json?

@Rajgupta36 no sadly we cannot do it as of now in this PR, as other components are still dependent on the react-tooltip the issue #645 addresses this, i've looked into it there

@arkid15r arkid15r enabled auto-merge January 28, 2025 19:35
@Rajgupta36
Copy link
Collaborator

Hi @Dishant1804, in this PR, we are using the Tooltip from Chakra instead of react-tooltip. Can we remove the react-tooltip dependency from package.json?

@Rajgupta36 no sadly we cannot do it as of now in this PR, as other components are still dependent on the react-tooltip the issue #645 addresses this, i've looked into it there

ohk

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

Merging this in, LGTM 👍

@arkid15r arkid15r added this pull request to the merge queue Jan 28, 2025
Merged via the queue into OWASP:main with commit 7eae0bd Jan 28, 2025
13 checks passed
@arkid15r arkid15r mentioned this pull request Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

project details card migration to chakra UI
3 participants