-
-
Notifications
You must be signed in to change notification settings - Fork 260
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 Collapsible Categories with Dropdown for Step 1 in Resume Builder #854
Conversation
…he step 1 section
WalkthroughThe changes in this pull request enhance the resume builder interface by updating the CSS for improved styling and adding functionality in the JavaScript files to manage user interactions. Key updates include the addition of collapsible sections in the HTML, event listeners for dynamically adding entries, and validation for user inputs. The overall structure remains intact, but the user experience is improved through visual feedback and interactive elements. Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
resume2.js (1)
1-2
: Improve file documentation.The comment is vague about the relationship with
collectResumeData.js
. Consider adding more detailed documentation about:
- The purpose of this file
- Its relationship with
collectResumeData.js
- The functionality it implements
-// this js file is part 2 for the collectResumeData.js +/** + * Resume Builder Step 1 - Collapsible Categories Implementation + * + * This file extends collectResumeData.js by adding dropdown functionality + * to manage the visibility of different resume sections (Personal Info, + * Education, Skills, etc.) in Step 1 of the resume builder. + */Resume.css (1)
Line range hint
1-300
: Consider adding responsive styles for collapsible sections.The current media queries only handle the progress bar responsiveness. For a complete responsive design, consider adding media queries for the collapsible sections.
Add these responsive styles:
@media (max-width: 768px) { .category-header { padding: 0.75rem; } .category-content { padding: 0 0.75rem; } /* Adjust form layout for better mobile experience */ .pInfo, .experience-entry, .project-entry { flex-direction: column; } .pInfoLeft, .pInfoRight, .expLeft, .expRight, .projectLeft, .projectRight { width: 100%; } }resume.html (2)
6-7
: Consider maintaining step context in title and updating Font Awesome.
- The title "Build Resume" loses the context of being Step 1 in a multi-step process. Consider keeping this context for better user orientation.
- Font Awesome 6.6.0 is not the latest stable version. Consider updating to the latest version for potential bug fixes and improvements.
- <title>Build Resume</title> + <title>Build Resume - Step 1</title> - <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="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.5.1/css/all.min.css" integrity="sha512-DTOQO9RWCH3ppGqcWaEA1BIZOC6xxalwEsw9c2QQeAIftl+Vegovlnee1c9QX4TctnWMn13TZye+giMm8e2LwA==" crossorigin="anonymous" referrerpolicy="no-referrer" />
283-283
: Improve script organization and naming.The current setup has two similarly named scripts (
Resume.js
andresume2.js
) which could lead to confusion. Consider:
- Using more descriptive names (e.g.,
main.js
andform-controls.js
)- Documenting the responsibility of each script
- Adding a comment about required loading order if dependencies exist
- <script src="Resume.js"></script> - <script src="resume2.js"></script> + <!-- Core functionality --> + <script src="main.js"></script> + <!-- Form controls and collapsible sections --> + <script src="form-controls.js"></script>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- Resume.css (1 hunks)
- Resume.js (0 hunks)
- resume.html (8 hunks)
- resume2.js (1 hunks)
💤 Files with no reviewable changes (1)
- Resume.js
🧰 Additional context used
🔇 Additional comments (3)
resume2.js (1)
1-2
: Verify integration with collectResumeData.jsSince this file is described as "part 2" for collectResumeData.js, we should verify the integration points between these files.
Resume.css (2)
161-161
: LGTM: Font weight adjustment for h3 headers.The font weight change to 400 for h3 headers provides a lighter, more modern appearance.
Line range hint
1-300
: Missing CSS styles for collapsible categories functionality.The PR objectives mention implementing collapsible categories with dropdown functionality, but the CSS file lacks the necessary styles to support this feature. Consider adding the following CSS to implement the collapsible functionality:
Add these styles to support the collapsible sections:
/* Collapsible section styles */ .category-header { display: flex; align-items: center; justify-content: space-between; cursor: pointer; padding: 1rem; background-color: #f8f9fa; border-radius: 4px; } .dropdown-icon { transition: transform 0.3s ease; } .dropdown-icon.expanded { transform: rotate(180deg); } .category-content { max-height: 0; overflow: hidden; transition: max-height 0.3s ease-out; } .category-content.expanded { max-height: 1000px; /* Adjust based on content */ transition: max-height 0.3s ease-in; } /* Optional: Add hover effect */ .category-header:hover { background-color: #e9ecef; }These styles will:
- Create a clickable header with proper spacing and visual feedback
- Add smooth transition animations for the dropdown icon rotation
- Implement expand/collapse animations using max-height
- Provide visual feedback on hover
Let's verify if the HTML structure exists to support these styles:
caret1.addEventListener('click',()=>{ | ||
if(pinfo.style.display === 'none'){ | ||
pinfo.style.display = ''; | ||
caret1.style.transform = 'rotate(180deg)'; | ||
} | ||
else{ | ||
pinfo.style.display = 'none'; | ||
caret1.style.transform = 'rotate(0)'; | ||
} | ||
}) | ||
caret2.addEventListener('click',()=>{ | ||
if(education.style.display === 'none'){ | ||
education.style.display = ''; | ||
edubtn.style.display = ''; | ||
caret2.style.transform = 'rotate(180deg)'; | ||
} | ||
else{ | ||
education.style.display = 'none'; | ||
edubtn.style.display = 'none'; | ||
caret2.style.transform = 'rotate(0)'; | ||
} | ||
}) | ||
caret3.addEventListener('click',()=>{ | ||
if(slabel.style.display === 'none'){ | ||
slabel.style.display = ''; | ||
sinput.style.display = ''; | ||
caret3.style.transform = 'rotate(180deg)'; | ||
} | ||
else{ | ||
slabel.style.display = 'none'; | ||
sinput.style.display = 'none'; | ||
caret3.style.transform = 'rotate(0)'; | ||
} | ||
}) | ||
caret4.addEventListener('click',()=>{ | ||
if(expfield.style.display === 'none'){ | ||
expfield.style.display = ''; | ||
expbtn.style.display = ''; | ||
caret4.style.transform = 'rotate(180deg)'; | ||
} | ||
else{ | ||
expfield.style.display = 'none'; | ||
expbtn.style.display = 'none'; | ||
caret4.style.transform = 'rotate(0)'; | ||
} | ||
}) | ||
caret5.addEventListener('click',()=>{ | ||
if(achive.style.display === 'none'){ | ||
achive.style.display = ''; | ||
caret5.style.transform = 'rotate(180deg)'; | ||
} | ||
else{ | ||
achive.style.display = 'none'; | ||
caret5.style.transform = 'rotate(0)'; | ||
} | ||
}) | ||
caret6.addEventListener('click',()=>{ | ||
if(projfield.style.display === 'none'){ | ||
projfield.style.display = ''; | ||
projbtn.style.display = ''; | ||
caret6.style.transform = 'rotate(180deg)'; | ||
} | ||
else{ | ||
projfield.style.display = 'none'; | ||
projbtn.style.display = 'none'; | ||
caret6.style.transform = 'rotate(0)'; | ||
} | ||
}) | ||
caret7.addEventListener('click',()=>{ | ||
if(lnglabel.style.display === 'none'){ | ||
lnglabel.style.display = ''; | ||
lnginput.style.display = ''; | ||
caret7.style.transform = 'rotate(180deg)'; | ||
} | ||
else{ | ||
lnglabel.style.display = 'none'; | ||
lnginput.style.display = 'none'; | ||
caret7.style.transform = 'rotate(0)'; | ||
} | ||
}) |
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.
🛠️ Refactor suggestion
Refactor event listeners to reduce duplication and improve accessibility.
The current implementation has several issues:
- Significant code duplication
- Direct style manipulation
- Missing accessibility features
- No smooth transitions
Consider refactoring the event listeners using a more maintainable approach:
const ROTATION_DURATION = '0.3s';
function toggleSection(sectionKey) {
const config = SECTIONS[sectionKey];
const caret = document.querySelector(config.caretId);
const content = document.querySelector(config.contentSelector);
const button = config.buttonSelector ? document.querySelector(config.buttonSelector) : null;
if (!caret || !content) return;
const isExpanded = content.style.display !== 'none';
// Update ARIA attributes
caret.setAttribute('aria-expanded', !isExpanded);
content.setAttribute('aria-hidden', isExpanded);
// Add CSS classes instead of inline styles
content.classList.toggle('hidden', isExpanded);
caret.classList.toggle('rotated', !isExpanded);
if (button) {
button.classList.toggle('hidden', isExpanded);
}
}
function initializeEventListeners() {
Object.keys(SECTIONS).forEach(sectionKey => {
const caret = document.querySelector(SECTIONS[sectionKey].caretId);
if (caret) {
// Add necessary ARIA attributes
caret.setAttribute('role', 'button');
caret.setAttribute('aria-expanded', 'false');
caret.setAttribute('tabindex', '0');
// Handle both click and keyboard events
caret.addEventListener('click', () => toggleSection(sectionKey));
caret.addEventListener('keydown', (e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
toggleSection(sectionKey);
}
});
}
});
}
Add these CSS classes:
.hidden {
display: none;
}
.rotated {
transform: rotate(180deg);
transition: transform ROTATION_DURATION ease;
}
[role="button"] {
cursor: pointer;
}
This refactoring:
- Reduces code duplication
- Improves accessibility with ARIA attributes
- Adds keyboard navigation
- Uses CSS classes instead of inline styles
- Adds smooth transitions
- Makes the code more maintainable
Consider implementing a state management solution (e.g., using a pub/sub pattern) if the application grows more complex, to better handle section states and their dependencies.
Issue Resolved:
Resolves issue #835: Add Collapsible Categories with Dropdown for Step 1 in Resume Builder.
Description:
This PR enhances the user interface of the Step 1 section in the resume builder by introducing collapsible categories with dropdown functionality. The primary goal is to reduce visual clutter and improve the overall user experience by allowing users to focus on one section at a time.
Feature Breakdown:
Collapsible Categories:
Hamburger Menu Style Functionality:
UI/UX Enhancements:
Screenshots:
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation