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 Cursor Animation of INDIAN TRICOLOR Theme #879

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 85 additions & 1 deletion Faqs.html
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,21 @@
justify-content: center;
}
</style>
</head>
<style>
/* circle styles */
.circle {
height: 24px;
width: 24px;
border-radius: 24px;
background-color: rgb(255, 0, 0);
position: fixed;
top: 0;
left: 0;
pointer-events: none;
z-index: 99999999;
}
</style>
Comment on lines +114 to +127
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

CSS styling needs alignment with Indian tricolor theme

The current circle styling has a few concerns:

  1. The hardcoded red background color doesn't align with the PR's objective of implementing an Indian tricolor theme
  2. The z-index value of 99999999 is unnecessarily high

Consider this improved implementation:

 .circle {
   height: 24px;
   width: 24px;
   border-radius: 24px;
-  background-color: rgb(255, 0, 0);
   position: fixed;
   top: 0;
   left: 0;
   pointer-events: none;
-  z-index: 99999999;
+  z-index: 9999;
 }

Note: The background-color is removed as it's being set dynamically in JavaScript.

Committable suggestion was skipped due to low confidence.

</head>

<body>
<header>
Expand Down Expand Up @@ -202,6 +216,76 @@ <h5>How do I download my resume?</h5>
});
});
</script>

<!-- Circles -->
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
Comment on lines +220 to +240
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve HTML structure and accessibility

The current implementation has several areas for improvement:

  1. The circle elements could be generated dynamically via JavaScript
  2. Missing accessibility considerations

Consider this improved approach:

-<!-- Circles -->
-<div class="circle"></div>
-<div class="circle"></div>
-... (repeated 18 more times)
+<div id="cursor-trail" aria-hidden="true">
+  <!-- Circles will be generated via JavaScript -->
+</div>

Then in your JavaScript:

const cursorTrail = document.getElementById('cursor-trail');
const circleCount = 20;

// Respect user's motion preferences
if (!window.matchMedia('(prefers-reduced-motion: reduce)').matches) {
  for (let i = 0; i < circleCount; i++) {
    const circle = document.createElement('div');
    circle.className = 'circle';
    cursorTrail.appendChild(circle);
  }
}


<script>
// coordinates for the cursor :
const coords = { x: 0, y: 0 };
const circles = document.querySelectorAll(".circle");

const colors = [
"#fc5720", "#fc6532", "#fc6532","#fd8b65", "#d5585c", "#fac3b1", "#f4d5cb", "#fefefe", "#fefefe", "#cdf2c5", "#b2eca5", "#95e283", "#7dda68", "#4adc2d", "#69d451", "#54c939", "#34b916", "#28b309", "#1e9c02",
];

circles.forEach(function (circle, index) {
circle.x = 0;
circle.y = 0;
circle.style.backgroundColor = colors[index % colors.length];
});

// update the coordinates when the mouse moves:
window.addEventListener("mousemove", function (e) {
coords.x = e.clientX;
coords.y = e.clientY;
});

// animation function to move the circles:
function animateCircles() {
let x = coords.x;
let y = coords.y;

circles.forEach(function (circle, index) {


circle.style.left = x - 12 + "px";
circle.style.top = y - 12 + "px";
circle.style.scale = (circles.length - index) / circles.length;

circle.x = x;
circle.y = y;

const nextCircle = circles[index + 1] || circles[0];
x += (nextCircle.x - x) * 0.3;
y += (nextCircle.y - y) * 0.3;
});

requestAnimationFrame(animateCircles);
}

animateCircles();
</script>
Comment on lines +242 to +287
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve animation implementation and performance

Several issues need attention:

  1. The colors array doesn't match the Indian tricolor theme
  2. Missing cleanup for animation frame
  3. Performance optimizations needed
  4. Empty line at 270

Consider these improvements:

 const colors = [
-  "#fc5720", "#fc6532", "#fc6532","#fd8b65", "#d5585c", "#fac3b1", "#f4d5cb", "#fefefe", "#fefefe", "#cdf2c5", "#b2eca5", "#95e283", "#7dda68", "#4adc2d", "#69d451", "#54c939", "#34b916", "#28b309", "#1e9c02", 
+  "#FF9933", // Saffron
+  "#FFFFFF", // White
+  "#138808", // Green
 ];

+let animationFrameId;
+
 function animateCircles() {
   // ... existing animation code ...
-  requestAnimationFrame(animateCircles);
+  animationFrameId = requestAnimationFrame(animateCircles);
 }

+// Cleanup on page unload
+window.addEventListener('unload', () => {
+  if (animationFrameId) {
+    cancelAnimationFrame(animationFrameId);
+  }
+});

+// Throttle animation for better performance
+let isThrottled = false;
+window.addEventListener("mousemove", (e) => {
+  if (!isThrottled) {
+    coords.x = e.clientX;
+    coords.y = e.clientY;
+    isThrottled = true;
+    setTimeout(() => isThrottled = false, 16); // ~60fps
+  }
+});

Committable suggestion was skipped due to low confidence.


</body>

</html>
89 changes: 87 additions & 2 deletions about.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.6.0/css/all.min.css" integrity="sha512-Kc323vGBEqzTmouAECnVceyQqyqdsSiqLQISBL29aUW4U/M7pSPA/gEUZQqv1cwx4OnYxTxve5UMg5GT6L4JJg==" crossorigin="anonymous" referrerpolicy="no-referrer" />
<link rel="stylesheet" href="style.css">
</head>


<style>
:root {
Expand Down Expand Up @@ -388,7 +388,22 @@
}
}
</style>
</head>

<style>
/* circle styles */
.circle {
height: 24px;
width: 24px;
border-radius: 24px;
background-color: rgb(255, 0, 0);
position: fixed;
top: 0;
left: 0;
pointer-events: none;
z-index: 99999999;
}
</style>
Comment on lines +393 to +405
Copy link

@coderabbitai coderabbitai bot Oct 25, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consolidate '.circle' Class Styles into External Stylesheet

The styles for the .circle class are defined within an inline <style> block. For consistency, it's advisable to move these styles to your external CSS file.

Add the .circle styles to your external CSS file:

.circle {
  height: 24px;
  width: 24px;
  border-radius: 24px;
  background-color: rgb(255, 0, 0);
  position: fixed;
  top: 0;
  left: 0;
  pointer-events: none;
  z-index: 99999999;
}

Copy link
Author

Choose a reason for hiding this comment

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

I checked by replacing all bunch of recomonded statements but the output changes and not what I need to have

Copy link

Choose a reason for hiding this comment

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

@PatelHarsh2006, understood. If moving the .circle styles to an external stylesheet changes the output and doesn't achieve the desired effect, it's fine to keep them inline to maintain the required functionality.

(_/)
( •_•)
/💧


✏️ Learnings added
Learnt from: PatelHarsh2006
PR: GarimaSingh0109/Resum-Resume#879
File: about.html:393-405
Timestamp: 2024-10-25T11:00:20.504Z
Learning: In `about.html`, moving the `.circle` class styles for the cursor animation to an external CSS file alters the output undesirably, so the styles need to remain inline within the `<style>` tag in the HTML file.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

</head>
<body>

<div id="progress-container">
Expand Down Expand Up @@ -542,5 +557,75 @@ <h3>Connect With Us</h3>
}
});
</script>

<!-- Circles -->
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>

<script>
// coordinates for the cursor :
const coords = { x: 0, y: 0 };
const circles = document.querySelectorAll(".circle");

const colors = [
"#fc5720", "#fc6532", "#fc6532","#fd8b65", "#d5585c", "#fac3b1", "#f4d5cb", "#fefefe", "#fefefe", "#cdf2c5", "#b2eca5", "#95e283", "#7dda68", "#4adc2d", "#69d451", "#54c939", "#34b916", "#28b309", "#1e9c02",
];
Comment on lines +588 to +590
Copy link

@coderabbitai coderabbitai bot Oct 25, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Align Circle Colors with the Indian Tricolor Theme

The colors array includes multiple colors, but to reflect the Indian tricolor theme accurately, it should primarily contain saffron, white, and green.

Modify the colors array to use the specific colors of the Indian flag:

-const colors = [
-  "#fc5720", "#fc6532", "#fc6532","#fd8b65", "#d5585c", "#fac3b1", "#f4d5cb", "#fefefe", "#fefefe", "#cdf2c5", "#b2eca5", "#95e283", "#7dda68", "#4adc2d", "#69d451", "#54c939", "#34b916", "#28b309", "#1e9c02", 
-];
+const colors = [
+  "#FF9933", // Saffron
+  "#FFFFFF", // White
+  "#138808", // Green
+];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const colors = [
"#fc5720", "#fc6532", "#fc6532","#fd8b65", "#d5585c", "#fac3b1", "#f4d5cb", "#fefefe", "#fefefe", "#cdf2c5", "#b2eca5", "#95e283", "#7dda68", "#4adc2d", "#69d451", "#54c939", "#34b916", "#28b309", "#1e9c02",
];
const colors = [
"#FF9933", // Saffron
"#FFFFFF", // White
"#138808", // Green
];

Copy link
Author

Choose a reason for hiding this comment

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

I checked by replacing all bunch of recomonded statements but the output changes and not what I need to have

Copy link

Choose a reason for hiding this comment

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

@PatelHarsh2006, I understand that replacing the colors array as suggested altered the output in a way that doesn't match your desired effect.

To maintain the animation while incorporating the Indian tricolor theme, you can create a gradient of colors that smoothly transition between saffron, white, and green. Here's a revised colors array:

const colors = [
  "#FF9933", // Saffron
  "#FFA347",
  "#FFAD5B",
  "#FFB76F",
  "#FFC184",
  "#FFCB98",
  "#FFD5AC",
  "#FFE0C0",
  "#FFEAD5",
  "#FFFFFF", // White
  "#D6F5D6",
  "#ADEBAD",
  "#85E085",
  "#5CD65C",
  "#33CC33",
  "#2EB82E",
  "#29A329",
  "#248F24",
  "#1F7A1F",
  "#138808", // Green
];

This array provides a smooth transition between the tricolor colors, which should enhance the visual effect of the animation. Make sure that the number of colors in the array corresponds well with the number of circles for a cohesive appearance.

Also, you might want to adjust the number of circles in your HTML to match the length of the colors array:

<!-- Circles -->
<div class="circle"></div>
<!-- Repeat the above line for as many times as there are colors -->

Let me know if this helps or if you'd like further assistance.

Copy link
Author

Choose a reason for hiding this comment

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

I don,t agree the colors recommonded as they does not give true feeling indian tricolor but what i agrre is the no of circle should match the color array

Copy link

Choose a reason for hiding this comment

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

@PatelHarsh2006, understood. Thank you for explaining your preference. Ensuring that the number of circles matches the length of the colors array is important for the animation to work properly. Let me know if you need any further assistance.

(。•́︿•̀。)


circles.forEach(function (circle, index) {
circle.x = 0;
circle.y = 0;
circle.style.backgroundColor = colors[index % colors.length];
});

// update the coordinates when the mouse moves:
window.addEventListener("mousemove", function (e) {
coords.x = e.clientX;
coords.y = e.clientY;
});

// animation function to move the circles:
function animateCircles() {
let x = coords.x;
let y = coords.y;

circles.forEach(function (circle, index) {


circle.style.left = x - 12 + "px";
circle.style.top = y - 12 + "px";
circle.style.scale = (circles.length - index) / circles.length;

circle.x = x;
circle.y = y;

const nextCircle = circles[index + 1] || circles[0];
x += (nextCircle.x - x) * 0.3;
y += (nextCircle.y - y) * 0.3;
});

requestAnimationFrame(animateCircles);
}

animateCircles();
</script>

</body>
</html>
86 changes: 85 additions & 1 deletion ats_score_finder.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,21 @@
margin-top: 10px;
}
</style>
</head>
<style>
/* circle styles */
.circle {
height: 24px;
width: 24px;
border-radius: 24px;
background-color: rgb(255, 0, 0);
position: fixed;
top: 0;
left: 0;
pointer-events: none;
z-index: 99999999;
}
</style>
Comment on lines +61 to +74
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Optimize circle styles for better maintainability.

The CSS styling for circles has several potential issues:

  1. The z-index value (99999999) is unnecessarily high
  2. Fixed positioning with pointer-events: none might affect accessibility

Consider these improvements:

 .circle {
   height: 24px;
   width: 24px;
   border-radius: 24px;
-  background-color: rgb(255, 0, 0);
   position: fixed;
   top: 0;
   left: 0;
   pointer-events: none;
-  z-index: 99999999;
+  z-index: 9999;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<style>
/* circle styles */
.circle {
height: 24px;
width: 24px;
border-radius: 24px;
background-color: rgb(255, 0, 0);
position: fixed;
top: 0;
left: 0;
pointer-events: none;
z-index: 99999999;
}
</style>
<style>
/* circle styles */
.circle {
height: 24px;
width: 24px;
border-radius: 24px;
position: fixed;
top: 0;
left: 0;
pointer-events: none;
z-index: 9999;
}
</style>

</head>
<body>
<div class="heading">
<br><br><br>
Expand Down Expand Up @@ -195,5 +209,75 @@ <h1>Resume ATS Score Finder</h1>
}
</script>


<!-- Circles -->
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>
<div class="circle"></div>

Comment on lines +213 to +234
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve circle elements implementation.

The current implementation has 20 identical div elements that could be optimized:

  1. Consider generating circles dynamically using JavaScript
  2. Add ARIA attributes for accessibility
  3. Reduce the number of elements for better performance

Consider this approach:

- <!-- Circles -->
- <div class="circle"></div>
- <!-- repeated 20 times -->
+ <!-- Cursor Animation Container -->
+ <div id="cursor-animation" aria-hidden="true">
+   <!-- Circles will be generated by JavaScript -->
+ </div>

And in your JavaScript:

const cursorAnimation = document.getElementById('cursor-animation');
const circleCount = 10; // Reduced number for better performance
for (let i = 0; i < circleCount; i++) {
    const circle = document.createElement('div');
    circle.className = 'circle';
    cursorAnimation.appendChild(circle);
}

<script>
// coordinates for the cursor :
const coords = { x: 0, y: 0 };
const circles = document.querySelectorAll(".circle");

const colors = [
"#fc5720", "#fc6532", "#fc6532","#fd8b65", "#d5585c", "#fac3b1", "#f4d5cb", "#fefefe", "#fefefe", "#cdf2c5", "#b2eca5", "#95e283", "#7dda68", "#4adc2d", "#69d451", "#54c939", "#34b916", "#28b309", "#1e9c02",
];

circles.forEach(function (circle, index) {
circle.x = 0;
circle.y = 0;
circle.style.backgroundColor = colors[index % colors.length];
});

// update the coordinates when the mouse moves:
window.addEventListener("mousemove", function (e) {
coords.x = e.clientX;
coords.y = e.clientY;
});

// animation function to move the circles:
function animateCircles() {
let x = coords.x;
let y = coords.y;

circles.forEach(function (circle, index) {


circle.style.left = x - 12 + "px";
circle.style.top = y - 12 + "px";
circle.style.scale = (circles.length - index) / circles.length;

circle.x = x;
circle.y = y;

const nextCircle = circles[index + 1] || circles[0];
x += (nextCircle.x - x) * 0.3;
y += (nextCircle.y - y) * 0.3;
});

requestAnimationFrame(animateCircles);
}

animateCircles();
</script>
Comment on lines +235 to +280
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Align implementation with Indian tricolor theme and optimize performance.

The current implementation has several areas for improvement:

  1. Colors don't match the Indian tricolor theme
  2. Missing performance optimizations
  3. No cleanup of animation resources

Consider these improvements:

 const colors = [
-  "#fc5720", "#fc6532", "#fc6532","#fd8b65", "#d5585c", "#fac3b1", "#f4d5cb", "#fefefe", "#fefefe", "#cdf2c5", "#b2eca5", "#95e283", "#7dda68", "#4adc2d", "#69d451", "#54c939", "#34b916", "#28b309", "#1e9c02", 
+  // Saffron shades
+  "#FF9933", "#FF9933", "#FFA346", "#FFA346",
+  // White shades
+  "#FFFFFF", "#FFFFFF", "#FFFFFF", "#FFFFFF",
+  // Green shades
+  "#138808", "#138808", "#138808", "#138808"
 ];

+ // Throttle mousemove events
+ let ticking = false;
 window.addEventListener("mousemove", function (e) {
+  if (!ticking) {
+    window.requestAnimationFrame(function() {
       coords.x = e.clientX;
       coords.y = e.clientY;
+      ticking = false;
+    });
+    ticking = true;
+  }
 });

+ // Store animation frame ID for cleanup
+ let animationFrameId;
 function animateCircles() {
   // ... existing animation code ...
-  requestAnimationFrame(animateCircles);
+  animationFrameId = requestAnimationFrame(animateCircles);
 }

+ // Cleanup on page unload
+ window.addEventListener('unload', function() {
+  if (animationFrameId) {
+    cancelAnimationFrame(animationFrameId);
+  }
+ });

Also, consider extracting magic numbers into named constants:

const CIRCLE_OFFSET = 12; // Replace hardcoded 12 in position calculations
const TRAIL_FACTOR = 0.3; // Replace hardcoded 0.3 in trail effect


</body>
</html>
Loading