-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Added animation in feature section #10208
Added animation in feature section #10208
Conversation
Hey @eddiejaoude |
Hey @eddiejaoude Please look into it i have made it as per your requirements. |
Hey @SaraJaoude @eddiejaoude It's been so long i have created this PR and no one has given me a response!! |
There was a problem hiding this 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
Hey @eddiejaoude Thank you for your response :)
|
Here is the solution of the features that i have mentioned above:
Here is the PR : #10321Biodrop.mp4 |
Hey @eddiejaoude 🙂 |
pages/index.js
Outdated
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`)} |
There was a problem hiding this comment.
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
Lines 8 to 12 in 090b58a
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", | |
)} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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(` | |||
${ |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
There was a problem hiding this 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" |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you 👍
982eb38
into
EddieHubCommunity:contributions
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 intoindex.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)
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