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

Added animation in feature section #10208

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Monilprajapati
Copy link

Closes #9874

Changes proposed

As we have mentioned in the Issue #9874 i have added the animation in the feature section without using any library such as framer-motion. it is completly made up with Tailwind CSS.

I have created one useElementOnscreen.jsx to observe the each feature card of the feature section to start the animation based on some threshold in viewport.
I have added keyframes and animate property in tailwind.config.js file to perform desired animations and finally i have made changes into index.js file which is containing this feature section where i made changes into conditional rendering part of class where the animation is changing based upon the even and odd feature.

Check List (Check all the applicable boxes)

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

Screenshots

Previews:
I think the video is lagging but it is my video recorder issue so please consider it else the animation is working well

Laptop screen:

laptop.mp4

Tablet screens:

tablet.mp4

Mobile screens:

mobile.mp4

@github-actions github-actions bot added the issue linked Pull Request has issue linked label Feb 1, 2024
@Monilprajapati
Copy link
Author

Hey @eddiejaoude

@Monilprajapati
Copy link
Author

Hey @eddiejaoude Please look into it i have made it as per your requirements.

@Monilprajapati
Copy link
Author

Hey @SaraJaoude @eddiejaoude It's been so long i have created this PR and no one has given me a response!!

pages/index.js Outdated Show resolved Hide resolved
pages/index.js Outdated Show resolved Hide resolved
Copy link
Member

@eddiejaoude eddiejaoude 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 the PR and reminder. I have reviewed and left some inline comments

@Monilprajapati
Copy link
Author

Hey @eddiejaoude Thank you for your response :)
One more thing i want to ask that i am also working on this #10209 issue and i am almost done with the solution. So could you please assign it to me!
Features that i am adding :

  1. Smooth Navbar animation : Sticky Navbar
  2. Sidebar drawer button in documentation page

@Monilprajapati
Copy link
Author

Monilprajapati commented Apr 3, 2024

Here is the solution of the features that i have mentioned above:

  • See the navbar animation and all
  • See the sidebar drawer menu button on bottom right to access it

Here is the PR : #10321

Biodrop.mp4

@Monilprajapati
Copy link
Author

Hey @eddiejaoude 🙂

@Monilprajapati
Copy link
Author

Hey @eddiejaoude @SaraJaoude

pages/index.js Outdated
Comment on lines 290 to 304
className={classNames(`
${
featureIdx % 2 === 0
? "lg:col-start-1"
: "lg:col-start-8 xl:col-start-9",
"mt-6 lg:mt-0 lg:row-start-1 lg:col-span-5 xl:col-span-4",
)}
? `lg:col-start-1 ${
featureRefs[featureIdx][1]
? "animate-fade-right"
: ""
}`
: `lg:col-start-8 xl:col-start-9 ${
featureRefs[featureIdx][1]
? "animate-fade-left"
: ""
}`
}
mt-6 lg:mt-0 opacity-0 lg:row-start-1 lg:col-span-5 xl:col-span-4`)}
Copy link
Member

Choose a reason for hiding this comment

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

This is very difficult to read, using classNames makes it easier as the backtics are not required and nested ternaries add to the complexity.

Here is an example

className={classNames(
"flex flex-col items-center border-2 h-[14rem] overflow-hidden rounded-lg shadow-lg transition duration-350 p-4 gap-3 duration-500 ease-in-out hover:border-tertiary-medium",
className,
active && "border-tertiary-medium",
)}

Copy link
Author

Choose a reason for hiding this comment

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

Hey @eddiejaoude I got your point but i don't understand that classNames so could you help me that how can i write that thing with classNames so that i can make those changes?

Copy link
Author

Choose a reason for hiding this comment

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

Hey @eddiejaoude I got your point but i don't understand that classNames so could you help me that how can i write that thing with classNames so that i can make those changes?

Okk Got it. Done with the changes

pages/index.js Outdated
@@ -299,12 +316,21 @@ export default function Home({
)}
</div>
<div
className={classNames(
className={classNames(`
${
Copy link
Member

Choose a reason for hiding this comment

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

Same here, ${ should not be required

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I got your point, but I am confused about how to use multiple conditions in classNames.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I got your point, but I am confused about how to use multiple conditions in classNames.

Solved!

Copy link
Member

@eddiejaoude eddiejaoude 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, I added some inline comments to make it more readable

: featureRefs[featureIdx][1] && featureIdx % 2 !== 0
? "animate-fade-left"
: "",
"opacity-0 lg:row-start-1 lg:col-span-5 xl:col-span-4"
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for making the changes, it is better but still difficult to read as indentation is not correct, if you add the Prettier plugin to VScode it will fix it all for you (btw this is also for other files in the PR)

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I always use it in general, but I was encountering some issues with that. See :

image

image

Copy link
Member

Choose a reason for hiding this comment

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

seems like a setup issue with your npm, maybe you used sudo previously? check the permissions are set to your current user

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

Successfully merging this pull request may close these issues.

[FEATURE] Animation in Feature section
2 participants