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

Cursor hover #218

Merged
merged 7 commits into from
Oct 22, 2023
Merged

Conversation

monisanees270798
Copy link
Contributor

Fixes Issue

Closes Issue #202

Changes proposed

List of changes proposed by my PR

  • Rewritten the entire tracks.tsx with easy to understand logic
  • Added feature to zoom in when cursor hovers over .svg images under batch section of HomePage
  • A supporting CSS file was created to do so (styles directory -> tracks.module.css)

Check List (Check all the applicable boxes)

  • [ x] My code follows the code style of this project.
  • [ x] My change requires changes to the documentation.
  • [ x] I have updated the documentation accordingly.
  • [ x] All new and existing tests passed.
  • [ x] This PR does not contain plagiarized content.
  • [ x] The title of my pull request is a short description of the requested changes.

Screenshots

image
image

Note to reviewers

I made sure my code does not create any bugs within the rest of your website. Please let me know if there are any issues with my PR getting accepted.

1. Rewrote the existing code and added additional hover code in an understandable way (tracks.tsx)
2. A supporting CSS file has been added to styles dir to support hover zoom
-----------------------------------------------
I will complete the .md file and initiate a PR
@vercel
Copy link

vercel bot commented Oct 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
frontendfreaks ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 21, 2023 0:17am

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for your pull request and welcome to our community! We will be getting back to you soon . Your patience will be greatly appreciated! Thanks! 🥳

Copy link
Member

@ManishBisht777 ManishBisht777 left a comment

Choose a reason for hiding this comment

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

Left some comments

@@ -0,0 +1,10 @@
/* components/Tracks.module.css */

.image-container {
Copy link
Member

Choose a reason for hiding this comment

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

can't we do it with tailwind only? instead of adding plain stylesheet?

Copy link
Contributor Author

@monisanees270798 monisanees270798 Oct 17, 2023

Choose a reason for hiding this comment

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

Yes definitely, I tried and it worked. Can you recheck the PR request?
@ManishBisht777 @Vishal-raj-1

1. Rewrote the existing code with tailwind CSS only (tracks.tsx)
2. The supporting CSS file has been removed from styles dir
-----------------------------------------------
Copy link
Member

@ManishBisht777 ManishBisht777 left a comment

Choose a reason for hiding this comment

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

Looks good just remove the unused imports

@@ -2,109 +2,52 @@ import Image from "next/image";
import { buttonVariants } from "./ui/button";
import Link from "next/link";
import { cn } from "@/lib/utils";
import React from "react"; // Import React
Copy link
Member

Choose a reason for hiding this comment

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

is there is need for this import?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I forgot to review imports after changing the CSS codes, I have removed it since we are working React above 17. Apologies for the overlook. Let me anything else required to complete the PR acceptance.
Thank you @ManishBisht777

Copy link
Member

Choose a reason for hiding this comment

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

There is some build error can you check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am trying to do, should I check it from Vercel or my local IDE?

1. Removed unnecessary imports from the code
-----------------------------------------------
@Vishal-raj-1
Copy link
Member

Hey @monisanees270798 any update why preview is failing again and again on this PR?

@monisanees270798
Copy link
Contributor Author

Hey @monisanees270798 any update why preview is failing again and again on this PR?

I am trying to learn it, I tried clicking on Vercel for the details but it keeps giving me error 404.
I am trying to fix it through my local repository, but if you can let me know what might be causing it would be helpful.

image

image

1. Trying to fix Deployment (Hover Effect only)
-----------------------------------------------
@monisanees270798
Copy link
Contributor Author

@ManishBisht777 @Vishal-raj-1
Cheers, all checks have been passed.
Please feel free to accept the PR
Many Thanks

Copy link
Member

@ManishBisht777 ManishBisht777 left a comment

Choose a reason for hiding this comment

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

LGTM :)

Copy link
Member

@Vishal-raj-1 Vishal-raj-1 left a comment

Choose a reason for hiding this comment

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

From next time, try not to change formatting of the file

@Vishal-raj-1 Vishal-raj-1 merged commit cf81f24 into FrontendFreaks:master Oct 22, 2023
3 checks passed
@monisanees270798
Copy link
Contributor Author

From next time, try not to change formatting of the file

Sure thing, it was a great learning experience. Thank You.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants