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

Add animtion card on idex page and and sparde borderand transition o… #827

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

Conversation

Mahendraish
Copy link

@Mahendraish Mahendraish commented Oct 22, 2024

…n about page

Pull Request for Resum-Resume 💡

Issue Title :

  • Info about the related issue (Aim of the project) :
  • Name:
  • GitHub ID:
  • Email ID:

Closes: #issue number that will be closed through this PR

Describe the add-ons or changes you've made 📃

Give a clear description of what have you added or modifications made

Type of change ☑️

What sort of change have you made:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, local variables)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested? ⚙️

Describe how it has been tested
Describe how have you verified the changes made

Checklist: ☑️

  • My code follows the guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly wherever it was hard to understand.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added things that prove my fix is effective or that my feature works.
  • Any dependent changes have been merged and published in downstream modules.

Summary by CodeRabbit

  • New Features

    • Enhanced styling and structure of the About, Index, and Signup pages for improved visual presentation.
    • Added CSS animations and hover effects for features cards, improving user interaction.
    • Introduced a scroll-to-top button for better navigation.
    • New GitHub link added in the footer for user connectivity.
    • Simplified signup form with improved validation and error handling.
  • Bug Fixes

    • Improved responsiveness for mobile devices, ensuring elements stack correctly on smaller screens.
  • Style

    • Updated colors, font sizes, and hover effects for a more engaging user experience.
    • Restructured layout with flexbox design for better organization and visual appeal.

Copy link

coderabbitai bot commented Oct 22, 2024

Walkthrough

The changes in this pull request involve significant updates to the about.html, index.html, and signup.html files, focusing on styling and structural enhancements. In about.html, CSS variables for colors have been defined, and various elements have received adjustments for improved visual presentation and responsiveness. In index.html, new CSS animations and hover effects have been introduced for feature cards, along with structural changes for better mobile responsiveness and user navigation improvements. The signup.html file has been updated with a streamlined layout and enhanced JavaScript functionality for user registration.

Changes

File Change Summary
about.html Updated CSS variables for colors, modified .about h1 font size and color, added italic style to paragraphs, enhanced .card class with transition effects, and made responsive adjustments for smaller screens.
index.html Added CSS animations and hover effects for .features-card, introduced @keyframes cardani, updated <h3> with an id, modified footer with a new GitHub link, and expanded script section for a scroll-to-top button.
signup.html Streamlined <head> section, updated styles for .navbar and .footer, simplified signup form, enhanced JavaScript for form validation and error handling, and updated logo text.

Possibly related PRs

Suggested labels

hacktoberfest, level2, gssoc-ext, hacktoberfest-accepted

Suggested reviewers

  • GarimaSingh0109

🐇 In the garden where colors bloom,
The about page sheds its gloom.
With hovers and styles, oh what a sight,
A scroll-to-top button brings delight!
Cards dance and sway, in vibrant hues,
A joyful update, for all to use! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Outside diff range and nitpick comments (4)
about.html (3)

361-364: Approve card hover effects with a minor optimization.

The added transition and hover effects for the .card class enhance user interaction and provide a polished look. Great job on implementing these visual improvements!

However, there's a minor optimization opportunity:

Consider removing the transition-delay: 0s; property on line 363, as 0s is the default value and doesn't affect the animation. This will make the code more concise:

.card {
    transition: all;
    transition-duration: 1s;
-   transition-delay: 0s;
}

Also applies to: 368-370


Line range hint 391-411: Approve progress bar implementation with a performance suggestion.

The addition of a progress bar to show scroll position is a great enhancement to the user experience. The implementation is well-done, using smooth animation for a polished look.

To further optimize performance, consider the following suggestion:

Use requestAnimationFrame directly instead of updating on every scroll event. This can help reduce unnecessary calculations, especially during rapid scrolling:

let lastScrollPercentage = 0;
let ticking = false;

function updateProgressBar() {
    const scrollTop = window.scrollY;
    const windowHeight = document.documentElement.scrollHeight - window.innerHeight;
    const scrollPercentage = (scrollTop / windowHeight) * 100;

    lastScrollPercentage += (scrollPercentage - lastScrollPercentage) * 0.1;
    
    const progressBar = document.getElementById('progress-bar');
    progressBar.style.width = lastScrollPercentage + '%';
    
    ticking = false;
}

window.addEventListener('scroll', function() {
    if (!ticking) {
        window.requestAnimationFrame(updateProgressBar);
        ticking = true;
    }
});

This change will make the progress bar update more efficiently, especially on mobile devices or during fast scrolling.


Line range hint 265-315: Approve responsive design with suggestions for navbar improvement.

The responsive design adjustments for smaller screens are well-implemented, ensuring the page is usable across various device sizes. Great job on adapting the layout for mobile views!

However, there's room for improvement in the navbar responsiveness:

  1. The current media query for the navbar starts at 768px, which might be too narrow for some tablet devices. Consider adjusting this breakpoint or adding an intermediate breakpoint.

  2. In the mobile view, the navbar items are stacked vertically, but there's no toggle mechanism to show/hide them. This could lead to a very tall header on mobile devices. Consider implementing a hamburger menu for mobile views.

Here's a suggestion for implementing a basic hamburger menu:

<nav class="navbar">
    <img src="images/logo.png" alt="logo">
    <h1>Resum Resume</h1>
    <button class="navbar-toggle"></button>
    <div class="navbar-links">
        <a class="tab" href="index.html">Home</a>
        <!-- ... other links ... -->
    </div>
</nav>
@media (max-width: 768px) {
    .navbar-toggle {
        display: block;
    }
    .navbar-links {
        display: none;
    }
    .navbar-links.active {
        display: flex;
        flex-direction: column;
    }
}
document.querySelector('.navbar-toggle').addEventListener('click', function() {
    document.querySelector('.navbar-links').classList.toggle('active');
});

This change will make the navbar more user-friendly on mobile devices while maintaining its current desktop appearance.

index.html (1)

Line range hint 49-100: Consolidate multiple <head> sections into a single <head>

Multiple <head> tags are present in the HTML document, which is invalid and can cause rendering issues. An HTML document should have only one <head> section. Merge the contents of all <head> sections into a single <head> at the top of the document.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 6b52130 and 373dd60.

📒 Files selected for processing (2)
  • about.html (3 hunks)
  • index.html (4 hunks)
🧰 Additional context used
🔇 Additional comments (3)
about.html (2)

Line range hint 1-563: Summary of review for about.html

Overall, the changes to about.html enhance the page's visual appeal, responsiveness, and user experience. Key improvements include:

  1. Updated styling for text elements
  2. Enhanced card hover effects
  3. Addition of a scroll progress bar
  4. More comprehensive content in the About section
  5. Responsive design adjustments for smaller screens

While these changes are generally positive, consider addressing the following points:

  1. Verify the responsiveness of the larger heading size
  2. Optimize the progress bar implementation for better performance
  3. Ensure the new heading color meets accessibility standards
  4. Improve the navbar's responsiveness, possibly by implementing a hamburger menu for mobile views

By addressing these points, you'll further enhance the page's usability and performance across different devices.


443-443: Verify accessibility of the new heading color.

The content updates in the About section and cards provide more comprehensive information about the LinkedIn Resume Builder, which is great for user understanding.

However, regarding the new heading color:

Please verify that the new color (#4c0359) for the "About" heading meets WCAG 2.1 color contrast requirements for accessibility. You can use online tools like WebAIM's Contrast Checker to ensure the color has sufficient contrast against the background color.

#!/bin/bash
# Extract background color of the about section
bg_color=$(rg -o 'background-color:\s*([^;]+)' about.html | head -n 1 | awk -F': ' '{print $2}')
echo "Background color: $bg_color"
echo "Heading color: #4c0359"
echo "Please use these colors in a contrast checker tool to verify accessibility."
index.html (1)

965-965: Verify the styling of the header element with id="resumec"

After correcting the CSS for #resumec, ensure that the <h3 id="resumec"> element reflects the intended styling in the browser. This verification is important to confirm that the changes have the desired visual impact.

@@ -337,13 +337,14 @@
padding-bottom: 5rem;
}
.about h1{
font-size: 4rem;
font-size: 5rem;
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Add media queries for .about h1 to ensure responsiveness.

Currently, there are no media queries adjusting the font size of .about h1, which may cause layout issues on smaller screens. Please consider adding responsive styles to modify the heading size for different screen widths.

🔗 Analysis chain

Verify responsiveness and readability of updated text styles.

The font size for .about h1 has been increased, and italic style has been added to .about p. While these changes may enhance visual appeal, please consider the following:

  1. Verify that the larger heading (5rem) doesn't cause layout issues on smaller screens.
  2. Assess the readability of the italic paragraph text, especially for longer content.

Consider adding media queries to adjust the heading size for smaller screens if necessary.

Also applies to: 347-347

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for existing media queries related to .about h1 font-size
rg -i "\.about\s*h1.*font-size" about.html

Length of output: 414


Script:

#!/bin/bash
# Search for media queries that adjust .about h1 font-size
rg -i '@media' about.html | rg -i '\.about\s*h1.*font-size'

Length of output: 59


Script:

#!/bin/bash
# Extract all media queries and search for any font-size adjustments within them
rg -i '@media' about.html -A 20 | rg -i 'font-size'

Length of output: 178

Comment on lines +65 to +95
@keyframes cardani {
10%{
border-color: #ff6f61;
}
20%{
border-color: #de8c3e;
}
30%{
border-color: brown;
}
40%{
border-color: aqua;
}

50%{
border-color: chartreuse;
}
60%{
border-color: burlywood;
}
70%{
border-color: red;
}

80%{
border-color: blue;
}

90%{
border-color: black;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Consolidate duplicate animations to avoid redundancy

The @keyframes cardani animation defined here appears to be duplicated later as @keyframes cardanii. This duplication can lead to increased file size and maintenance challenges. Consider consolidating these animations into a single definition if they are intended to perform the same effect.

Comment on lines +930 to +932
#resumec{
columns: rgb(20, 198, 8);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the invalid CSS property in #resumec

The CSS property columns: rgb(20, 198, 8); is invalid, as columns is used for multi-column layouts, not for setting colors. If the intention is to change the text color, you should use the color property.

Apply this diff to fix the property:

#resumec {
-  columns: rgb(20, 198, 8);
+  color: rgb(20, 198, 8);
}
📝 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
#resumec{
columns: rgb(20, 198, 8);
}
#resumec{
color: rgb(20, 198, 8);
}

Comment on lines +59 to +64
animation-name: cardani;
animation-duration: 1s;
animation-delay: 0s;
animation-iteration-count: infinite;
border: 2px solid red;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Refine the border animation in .features-card:hover

The border: 2px solid red; in the :hover state sets a static red border, but the subsequent animation changes the border-color over time. This may cause a visual inconsistency at the start of the hover effect. Consider setting the initial border-color to match the starting color of the animation or remove the static border color to allow the animation to transition smoothly.

Apply this diff to adjust the border styling:

.features-card:hover {
  transform: translateY(-10px) scale(1.1);
  box-shadow: 0 4px 8px #0073b1;
  z-index: 3;
  animation-name: cardani;
  animation-duration: 1s;
  animation-delay: 0s;
  animation-iteration-count: infinite;
-  border: 2px solid red;
+  border: 2px solid #ff6f61; /* Match starting color of animation */
}
📝 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
animation-name: cardani;
animation-duration: 1s;
animation-delay: 0s;
animation-iteration-count: infinite;
border: 2px solid red;
}
animation-name: cardani;
animation-duration: 1s;
animation-delay: 0s;
animation-iteration-count: infinite;
border: 2px solid #ff6f61; /* Match starting color of animation */
}

Comment on lines +308 to +345
border: 2px solid grey;
animation-name: cardanii;
animation-duration: 2s;
animation-delay:0s;
animation-iteration-count: infinite;
}
@keyframes cardanii {
10%{
border-color: #ff6f61;
}
20%{
border-color: #de8c3e;
}
30%{
border-color: brown;
}
40%{
border-color: aqua;
}

50%{
border-color: chartreuse;
}
60%{
border-color: burlywood;
}
70%{
border-color: red;
}

80%{
border-color: blue;
}

90%{
border-color: black;
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Remove redundant @keyframes cardanii animation

The @keyframes cardanii defined here duplicates the earlier @keyframes cardani. To adhere to the DRY (Don't Repeat Yourself) principle, remove the redundant animation or ensure that both animations are intentionally different.

Apply this diff to remove the redundant animation:

- @keyframes cardanii {
-   /* Duplicate animation steps */
- }

Committable suggestion was skipped due to low confidence.

@Mahendraish
Copy link
Author

Add styling on fotter

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (1)
signup.html (1)

Line range hint 70-140: Add form validation styles for better user feedback.

The form lacks visual feedback for invalid inputs and error states.

Add these styles for better user experience:

     input[type="text"],
     input[type="email"],
     input[type="password"] {
         width: 100%;
         padding: 12px;
         margin: 10px 0;
         border: 1px solid #ccc;
         border-radius: 4px;
         font-size: 1rem;
+        transition: border-color 0.3s ease;
+    }
+
+    input:invalid {
+        border-color: #ff4444;
+    }
+
+    input:focus {
+        outline: none;
+        border-color: var(--deep-teal);
+        box-shadow: 0 0 0 2px rgba(0, 61, 77, 0.2);
     }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 373dd60 and 42bd830.

📒 Files selected for processing (1)
  • signup.html (4 hunks)
🔇 Additional comments (1)
signup.html (1)

308-371: ⚠️ Potential issue

Add security measures to prevent common attacks.

The current implementation lacks several security measures:

  1. No rate limiting for form submission
  2. No input sanitization
  3. Passwords transmitted in plain text

Let's verify if proper security headers are set:

Add these security enhancements:

+    let submitAttempts = 0;
+    const MAX_ATTEMPTS = 5;
+    const COOLDOWN_PERIOD = 300000; // 5 minutes in milliseconds
+
+    function sanitizeInput(input) {
+        return input.replace(/[<>]/g, ''); // Basic XSS prevention
+    }
+
+    function isRateLimited() {
+        if (submitAttempts >= MAX_ATTEMPTS) {
+            const remainingTime = Math.ceil(COOLDOWN_PERIOD / 1000 / 60);
+            alert(`Too many attempts. Please try again in ${remainingTime} minutes.`);
+            return true;
+        }
+        return false;
+    }
+
     async function handleFormSubmission(event) {
         event.preventDefault();
+        
+        if (isRateLimited()) {
+            return;
+        }
+
         const form = event.target;
         const formData = new FormData(form);
+        submitAttempts++;
+
+        // Reset attempts after cooldown period
+        setTimeout(() => {
+            submitAttempts = Math.max(0, submitAttempts - 1);
+        }, COOLDOWN_PERIOD);

         try {
+            // Sanitize inputs
+            const sanitizedData = Object.fromEntries(
+                Array.from(formData.entries()).map(([key, value]) => [
+                    key,
+                    typeof value === 'string' ? sanitizeInput(value) : value
+                ])
+            );
+
             const password = formData.get('password');
             const confirmPassword = formData.get('confirm_password');

             validatePassword(password);

<div class="footer-section">
<h3>Support</h3>
<ul>
<li><a href="#">FAQ</a></ <li><a href="#">Contact Us</a></li>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix broken HTML tag in footer support section.

There's a syntax error in the FAQ link tag.

Fix the broken HTML:

-    <li><a href="#">FAQ</a></ <li><a href="#">Contact Us</a></li>
+    <li><a href="#">FAQ</a></li>
+    <li><a href="#">Contact Us</a></li>
📝 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
<li><a href="#">FAQ</a></ <li><a href="#">Contact Us</a></li>
<li><a href="#">FAQ</a></li>
<li><a href="#">Contact Us</a></li>

Comment on lines +33 to +69
.navbar {
background-color: var(--deep-teal);
padding: 20px;
display: flex;
justify-content: space-between;
align-items: center;
}

.navbar h1 {
color: var(--off-white);
margin: 0;
font-size: 1.5rem;
transition: color 0.3s ease;
}

.navbar h1:hover {
color: var(--soft-gold);
}

.navbar ul {
list-style-type: none;
margin: 0;
padding: 0;
display: flex;
justify-content: center;
align-items: center;
height: 100vh;
flex-direction: column;
} */
body {
background-color: var(--off-white);
margin: 0;
padding: 0;
min-height: 100vh;
display:flex;
flex-direction: column;
}
gap: 20px;
}

.navbar ul li a {
color: var(--off-white);
text-decoration: none;
font-size: 16px;
}

.navbar ul li a:hover {
color: var(--soft-gold);
transition: color 0.3s ease;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add ARIA labels and keyboard navigation support to navbar.

The navbar needs accessibility improvements:

  • Add role="navigation" to the nav element
  • Add aria-current="page" to indicate the active page
  • Ensure keyboard focus styles match hover styles

Apply these changes to improve accessibility:

-    <nav class="navbar">
+    <nav class="navbar" role="navigation" aria-label="Main navigation">

     .navbar ul li a:hover,
+    .navbar ul li a:focus {
+        color: var(--soft-gold);
+        transition: color 0.3s ease;
+        outline: 2px solid var(--soft-gold);
+        outline-offset: 2px;
     }

Committable suggestion was skipped due to low confidence.

Comment on lines 258 to 265
<form id="signupForm" action="#" method="POST">
<input type="text" placeholder="Full Name" required>
<input type="email" placeholder="Email Address" required>
<input type="password" placeholder="Password" required>
<input type="password" placeholder="Confirm Password" required>
<button type="submit">Sign Up</button>

</form>
<button class="login-btn" onclick="window.location.href='login.html'">Go to Login</button>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance form accessibility and security.

The form needs improvements in accessibility and security:

  • Missing name attributes on inputs
  • Lacking ARIA labels
  • No CSRF protection

Apply these changes:

-    <form id="signupForm" action="#" method="POST">
+    <form id="signupForm" action="#" method="POST" aria-label="Sign up form">
+        <input type="hidden" name="csrf_token" value="<?php echo generate_csrf_token(); ?>">
-        <input type="text" placeholder="Full Name" required>
+        <input type="text" name="fullname" placeholder="Full Name" required aria-label="Full Name">
-        <input type="email" placeholder="Email Address" required>
+        <input type="email" name="email" placeholder="Email Address" required aria-label="Email Address">
-        <input type="password" placeholder="Password" required>
+        <input type="password" name="password" placeholder="Password" required aria-label="Password" minlength="8">
-        <input type="password" placeholder="Confirm Password" required>
+        <input type="password" name="confirm_password" placeholder="Confirm Password" required aria-label="Confirm Password" minlength="8">

Committable suggestion was skipped due to low confidence.

Comment on lines 308 to 371
<script>
const signUp = document.querySelector(".signupbtn");

signUp.addEventListener("click", (e) => {
e.preventDefault();

const name = document.querySelector("#name").value;
const email = document.querySelector("#email").value;
const password = document.querySelector("#password").value;
const confirmPassword = document.querySelector("#confirmPassword").value;

if (!name || !email || !password || !confirmPassword) {
alert("All fields are required");
return;
}

const userData = {
name,
email,
password,
confirmPassword
};

registerUser(userData);
});

const registerUser = async (user) => {
try {
const res = await fetch("http://localhost:5000/auth/signup", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify(user)
});

const data = await res.json();
window.location.href = "http://127.0.0.1:5500/Resum-Resume/login.html";

} catch (error) {
console.log(error.message);
alert("Check the credentials");
}
};

document.getElementById('signupForm').addEventListener('submit', function (event) {
event.preventDefault();

var formData = new FormData(this);

fetch('signup-process.php', {
method: 'POST',
body: formData
})
.then(response => response.json())
.then(data => {
if (data.success) {
window.location.href = 'index.html';
} else {
alert('Sign Up failed: ' + data.message);
}
})
.catch(error => {
console.log('Error:', error);
});
.then(response => response.json())
.then(data => {
if (data.success) {
window.location.href = 'index.html';
} else {
alert('Sign Up failed: ' + data.message);
}
})
.catch(error => {
console.log('Error:', error);
});
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Refactor JavaScript to fix multiple issues.

The JavaScript implementation has several critical issues:

  1. Duplicate form submission logic
  2. Hardcoded URLs
  3. Missing password validation
  4. Inadequate error handling

Here's a refactored version:

-    const signUp = document.querySelector(".signupbtn");
-    signUp.addEventListener("click", (e) => {
-        e.preventDefault();
-        const name = document.querySelector("#name").value;
-        const email = document.querySelector("#email").value;
-        const password = document.querySelector("#password").value;
-        const confirmPassword = document.querySelector("#confirmPassword").value;
-        // ... rest of the first event listener
-    });

+    const API_BASE_URL = 'http://localhost:5000';
+    const REDIRECT_URL = 'login.html';
+
+    function validatePassword(password) {
+        const minLength = 8;
+        const hasUpperCase = /[A-Z]/.test(password);
+        const hasLowerCase = /[a-z]/.test(password);
+        const hasNumbers = /\d/.test(password);
+        
+        if (password.length < minLength) {
+            throw new Error('Password must be at least 8 characters long');
+        }
+        if (!hasUpperCase || !hasLowerCase || !hasNumbers) {
+            throw new Error('Password must contain uppercase, lowercase letters and numbers');
+        }
+    }
+
+    async function handleFormSubmission(event) {
+        event.preventDefault();
+        const form = event.target;
+        const formData = new FormData(form);
+
+        try {
+            const password = formData.get('password');
+            const confirmPassword = formData.get('confirm_password');
+
+            validatePassword(password);
+
+            if (password !== confirmPassword) {
+                throw new Error('Passwords do not match');
+            }
+
+            const response = await fetch(`${API_BASE_URL}/auth/signup`, {
+                method: 'POST',
+                headers: {
+                    'Content-Type': 'application/json',
+                    'X-CSRF-Token': formData.get('csrf_token')
+                },
+                body: JSON.stringify(Object.fromEntries(formData))
+            });
+
+            if (!response.ok) {
+                const error = await response.json();
+                throw new Error(error.message || 'Signup failed');
+            }
+
+            window.location.href = REDIRECT_URL;
+        } catch (error) {
+            console.error('Signup error:', error);
+            alert(error.message);
+        }
+    }
+
+    document.getElementById('signupForm').addEventListener('submit', handleFormSubmission);
📝 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
<script>
const signUp = document.querySelector(".signupbtn");
signUp.addEventListener("click", (e) => {
e.preventDefault();
const name = document.querySelector("#name").value;
const email = document.querySelector("#email").value;
const password = document.querySelector("#password").value;
const confirmPassword = document.querySelector("#confirmPassword").value;
if (!name || !email || !password || !confirmPassword) {
alert("All fields are required");
return;
}
const userData = {
name,
email,
password,
confirmPassword
};
registerUser(userData);
});
const registerUser = async (user) => {
try {
const res = await fetch("http://localhost:5000/auth/signup", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify(user)
});
const data = await res.json();
window.location.href = "http://127.0.0.1:5500/Resum-Resume/login.html";
} catch (error) {
console.log(error.message);
alert("Check the credentials");
}
};
document.getElementById('signupForm').addEventListener('submit', function (event) {
event.preventDefault();
var formData = new FormData(this);
fetch('signup-process.php', {
method: 'POST',
body: formData
})
.then(response => response.json())
.then(data => {
if (data.success) {
window.location.href = 'index.html';
} else {
alert('Sign Up failed: ' + data.message);
}
})
.catch(error => {
console.log('Error:', error);
});
.then(response => response.json())
.then(data => {
if (data.success) {
window.location.href = 'index.html';
} else {
alert('Sign Up failed: ' + data.message);
}
})
.catch(error => {
console.log('Error:', error);
});
});
<script>
const API_BASE_URL = 'http://localhost:5000';
const REDIRECT_URL = 'login.html';
function validatePassword(password) {
const minLength = 8;
const hasUpperCase = /[A-Z]/.test(password);
const hasLowerCase = /[a-z]/.test(password);
const hasNumbers = /\d/.test(password);
if (password.length < minLength) {
throw new Error('Password must be at least 8 characters long');
}
if (!hasUpperCase || !hasLowerCase || !hasNumbers) {
throw new Error('Password must contain uppercase, lowercase letters and numbers');
}
}
async function handleFormSubmission(event) {
event.preventDefault();
const form = event.target;
const formData = new FormData(form);
try {
const password = formData.get('password');
const confirmPassword = formData.get('confirm_password');
validatePassword(password);
if (password !== confirmPassword) {
throw new Error('Passwords do not match');
}
const response = await fetch(`${API_BASE_URL}/auth/signup`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-CSRF-Token': formData.get('csrf_token')
},
body: JSON.stringify(Object.fromEntries(formData))
});
if (!response.ok) {
const error = await response.json();
throw new Error(error.message || 'Signup failed');
}
window.location.href = REDIRECT_URL;
} catch (error) {
console.error('Signup error:', error);
alert(error.message);
}
}
document.getElementById('signupForm').addEventListener('submit', handleFormSubmission);
</script>

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.

1 participant