Update: Refactor UI/UX styles, enhance JS code consistency, and improve documentation formatting#22
Conversation
…ve documentation formatting
…ve documentation formatting
…ve documentation formatting
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
✅ Deploy Preview for fullstack-learning-management-system canceled.
|
Summary of ChangesHello @hoangsonww, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing code consistency and maintainability across the project. It primarily standardizes JavaScript string literals to use double quotes, updates the Prettier configuration to ignore Markdown files, and includes minor CSS formatting improvements. These changes are purely stylistic and do not introduce new features or alter existing application behavior. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on code formatting and minor refactoring across packages/script.js and packages/styles.css. In script.js, changes include converting single quotes to double quotes, reformatting array definitions, and improving readability of console.log statements. In styles.css, the changes involve reformatting CSS properties and linear-gradient declarations for better readability, and adjusting the padding of .tab-btn. The LICENSE file was updated to reflect a 2025 copyright, and *.md files were added to .prettierignore. The review comments highlight two key areas for improvement in script.js: the initSearch function has a logical flaw where it incorrectly hides parent containers based on individual child elements not matching, and a refactor is suggested to correctly filter based on the container's overall text content; additionally, the initCopyButtons function directly manipulates button.style.background, and the reviewer suggests using CSS classes (.copied, .error) for managing button visual states to improve maintainability and separation of concerns.
| const searchableElements = document.querySelectorAll( | ||
| "section, .card, h2, h3, p", | ||
| ); | ||
|
|
||
| searchInput.addEventListener("input", (e) => { | ||
| const searchTerm = e.target.value.toLowerCase(); | ||
| searchableElements.forEach(element => { | ||
|
|
||
| searchableElements.forEach((element) => { | ||
| const text = element.textContent.toLowerCase(); | ||
| const parent = element.closest('.card, section'); | ||
| if (text.includes(searchTerm) || searchTerm === '') { | ||
| if (parent) parent.style.display = ''; | ||
| element.style.display = ''; | ||
| const parent = element.closest(".card, section"); | ||
|
|
||
| if (text.includes(searchTerm) || searchTerm === "") { | ||
| if (parent) parent.style.display = ""; | ||
| element.style.display = ""; | ||
| } else { | ||
| if (parent) parent.style.display = 'none'; | ||
| if (parent) parent.style.display = "none"; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The current search implementation has a logic flaw that can cause incorrect filtering. When a search term doesn't match an element (like a <p>), it hides the element's parent (.card or section). This can hide a parent container even if other child elements within it do match the search term, because the loop will continue and a non-matching child will hide the parent, leading to an incorrect final state.
To fix this, you should iterate through the main content blocks (e.g., section or .card) and decide whether to show or hide each block based on whether its text content contains the search term. This ensures that a block is visible if any part of it matches.
const searchableContainers = document.querySelectorAll("section[id], .card");
searchInput.addEventListener("input", (e) => {
const searchTerm = e.target.value.toLowerCase();
searchableContainers.forEach((container) => {
const text = container.textContent.toLowerCase();
const isMatch = text.includes(searchTerm) || searchTerm === "";
container.style.display = isMatch ? "" : "none";
});
});| try { | ||
| await navigator.clipboard.writeText(code); | ||
| const originalText = button.textContent; | ||
| button.textContent = '✓ Copied!'; | ||
| button.style.background = '#10b981'; | ||
| button.textContent = "✓ Copied!"; | ||
| button.style.background = "#10b981"; | ||
|
|
||
| setTimeout(() => { | ||
| button.textContent = originalText; | ||
| button.style.background = ''; | ||
| button.style.background = ""; | ||
| }, 2000); | ||
| } catch (err) { | ||
| console.error('Failed to copy code:', err); | ||
| button.textContent = '✗ Error'; | ||
| button.style.background = '#ef4444'; | ||
| console.error("Failed to copy code:", err); | ||
| button.textContent = "✗ Error"; | ||
| button.style.background = "#ef4444"; | ||
|
|
||
| setTimeout(() => { | ||
| button.textContent = 'Copy'; | ||
| button.style.background = ''; | ||
| button.textContent = "Copy"; | ||
| button.style.background = ""; | ||
| }, 2000); | ||
| } |
There was a problem hiding this comment.
Directly manipulating button.style.background mixes JavaScript logic with CSS styling. For better maintainability and separation of concerns, it's recommended to use CSS classes to handle visual states.
You can define .copied and .error classes in your stylesheet and simply add or remove them from the button in JavaScript.
Example CSS:
.copy-btn.copied {
background-color: var(--success-color);
}
.copy-btn.error {
background-color: var(--danger-color);
}| try { | |
| await navigator.clipboard.writeText(code); | |
| const originalText = button.textContent; | |
| button.textContent = '✓ Copied!'; | |
| button.style.background = '#10b981'; | |
| button.textContent = "✓ Copied!"; | |
| button.style.background = "#10b981"; | |
| setTimeout(() => { | |
| button.textContent = originalText; | |
| button.style.background = ''; | |
| button.style.background = ""; | |
| }, 2000); | |
| } catch (err) { | |
| console.error('Failed to copy code:', err); | |
| button.textContent = '✗ Error'; | |
| button.style.background = '#ef4444'; | |
| console.error("Failed to copy code:", err); | |
| button.textContent = "✗ Error"; | |
| button.style.background = "#ef4444"; | |
| setTimeout(() => { | |
| button.textContent = 'Copy'; | |
| button.style.background = ''; | |
| button.textContent = "Copy"; | |
| button.style.background = ""; | |
| }, 2000); | |
| } | |
| try { | |
| await navigator.clipboard.writeText(code); | |
| const originalText = button.textContent; | |
| button.textContent = "✓ Copied!"; | |
| button.classList.add("copied"); | |
| setTimeout(() => { | |
| button.textContent = originalText; | |
| button.classList.remove("copied"); | |
| }, 2000); | |
| } catch (err) { | |
| console.error("Failed to copy code:", err); | |
| button.textContent = "✗ Error"; | |
| button.classList.add("error"); | |
| setTimeout(() => { | |
| button.textContent = "Copy"; | |
| button.classList.remove("error"); | |
| }, 2000); | |
| } |
This pull request focuses on standardizing the use of double quotes in the JavaScript codebase and updating the
.prettierignorefile. The main changes ensure consistent code style and improve maintainability, but do not introduce new features or alter application behavior.Code style consistency:
packages/script.jsto enforce a uniform quoting style. This affects DOM queries, event listener names, class and ID selectors, and string literals in all functions. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18]Configuration updates:
*.md) in the.prettierignorefile, preventing Prettier from formatting documentation or markdown files.