Skip to content
OverviewSolutions
19
Comments
19

Habeeb Kareem

@olaide-hokLagos, Nigeria470 points

A fullstack web developer.

Latest solutions

View more solutions

Latest comments

  • P
    Habeeb Kareem•470
    @olaide-hokPosted 2 days ago

    Great work here!

    The UI looks really good.

    Some areas to review would be:

    1. Repeated Code: There is repeated code for deselecting thumbnails and updating the carousel slide. You can create reusable functions for common tasks. For example
    function deselectAllThumbnails(container) {
    for (const image of Object.values(container.children)) {
        image.classList.remove("thumbnail-selected");
    }
}

// Usage
deselectAllThumbnails(thumbnailEl);
deselectAllThumbnails(secondThumbnailEl);
    1. Accessibility: The code does not consider accessibility (e.g., keyboard navigation, ARIA attributes). You can add ARIA attributes and ensure all interactive elements are keyboard accessible.

    For example:

    nextBtn.setAttribute("aria-label", "Next slide");
prevBtn.setAttribute("aria-label", "Previous slide");
    1. Performance Optimization: The resizeHandler function is called frequently during window resizing, which can cause performance issues.You can use debouncing to limit the number of times the function is called. E.g
    function debounce(func, delay) {
    let timeout;
    return function () {
        clearTimeout(timeout);
        timeout = setTimeout(() => func.apply(this, arguments), delay);
    };
}

window.addEventListener("resize", debounce(resizeHandler, 100));
    1. Code Comments: Some parts of the code lack comments, making it harder to understand the purpose of certain logic. Consider adding comments to explain complex or non-obvious logic.

    Overall well done!

    Marked as helpful
    0
  • P
    Habeeb Kareem•470
    @olaide-hokPosted 8 days ago

    Great work!

    Some areas to improve on are:

    • there is an overflow on mobile view.

    • CSS Reset: the reset is overly verbose and includes many elements that may not be necessary. Consider using a more concise reset like Normalize.css or Modern CSS Reset. Alternatively, use a tool like PostCSS to automate resetting only the elements you use. I personally use Normalize.css.

    • Repetitive Media Queries: the media queries are repeated for similar breakpoints (e.g. $tablet-breakpoint, $desktop-breakpoint). This can be streamlined using mixins or utility classes. You could create a mixin for common breakpoints for example

    @mixin for-tablet {
    @media screen and (min-width: $tablet-breakpoint) {
        @content;
    }
}

@mixin for-desktop {
    @media screen and (min-width: $desktop-breakpoint) {
        @content;
    }
}

@mixin for-big-screen {
    @media screen and (min-width: $big-screen-breakpoint) {
        @content;
    }
}

    and you could use like

    header {
    padding: 2rem 1rem 1rem;
    max-width: $tablet-breakpoint;

    @include for-tablet {
        max-width: $desktop-breakpoint;
    }

    @include for-desktop {
        max-width: $big-screen-breakpoint;
        padding: 2rem 2rem;
    }
}
    • Lack of Utility Classes: there are repeated styles (e.g., display: flex, gap: 1rem) could be abstracted into utility classes for reusability. You could create utility classes such as:
    .flex {
    display: flex;
}

.flex-col {
    flex-direction: column;
}

.gap-1 {
    gap: 1rem;
}

.gap-2 {
    gap: 2rem;
}

    and usage would be:

    #newsContainer {
    @extend .flex, .flex-col, .gap-1;
}

    • the font-size for the h1 max-value is 56px, the clamp values can be updated.

    • the spacings as well could be updated for large screens.

    Overall, great work, well done.

    0
  • P
    Habeeb Kareem•470
    @olaide-hokPosted 11 days ago

    Great job here!

    Some areas to improve the UI are:

    1. The checkbox and radio input type color can be updated by adding css rule to the selector below:
    .container form .query-control .radio-group input {
accent-color: #0c7d69;
}
    1. The radio-group can be wrapped in a single div and given a display of flex with a flex-direction set to column for mobile and on tablet and above viewport, it is then set to row. This approach can be utilized for the first and last name input fields as well.

    For your JS:

    1. Code Duplication: There is repeated code for adding/removing the error class and toggling error messages. This can be refactored into reusable functions. You can create helper functions for adding/removing errors:
    const showError = (element, errorElement) => {
  element.classList.add("error");
  errorElement.classList.remove("d-none");
};

const hideError = (element, errorElement) => {
  element.classList.remove("error");
  errorElement.classList.add("d-none");
};

    Use these functions to simplify the validation logic:

    if (!fName.value) {
  showError(fName, document.querySelector(`#fname + span`));
  success = false;
}
    1. Email Validation: The email validation regex (/^\w+@[a-zA-Z_]+.[a-zA-Z]{2,3}$/) is too restrictive. It doesn’t account for valid email formats like user.name+tag@domain.co.uk. You can use a more robust regex for email validation:
    const emailPattern = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
    1. Error Message Handling: The code does not clear all errors when the form is reset or when the user starts typing again. You can create a function to clear all errors such as:
    const clearErrors = () => {
  const errorElements = document.querySelectorAll(".error-message");
  errorElements.forEach((element) => element.classList.add("d-none"));

  const inputElements = document.querySelectorAll("input, textarea");
  inputElements.forEach((element) => element.classList.remove("error"));
};

    Then call this function in the change event listener:

    myForm.addEventListener("change", clearErrors);

    Overall, well done!

    0
  • P
    Habeeb Kareem•470
    @olaide-hokPosted 14 days ago

    Well done!

    Some areas to improve would include updating the css of some selectors:

    .accordion {
	font-family: Work Sans;
}

section {
    border-bottom: 1px solid var(--Light-Pink);
}

section .accordion:hover {
    color: #AD28EB;
}

section p {
    color: #8B6990;
}

    On the JS side,

    1. Performance: Your code uses document.getElementsByClassName, which returns a live HTMLCollection. This can cause performance issues if the DOM changes frequently. Use document.querySelectorAll instead, which returns a static NodeList:
    let acc = document.querySelectorAll(".accordion");
    1. Avoid Inline Styles: The code directly manipulates the style.display property of the panel. This approach is not ideal because it mixes JavaScript with presentation logic, making it harder to maintain and override styles with CSS. Consider using CSS classes to control visibility and transitions.

    For example:

    .panel {
  display: none;
  transition: max-height 0.3s ease-out;
  overflow: hidden;
  max-height: 0;
}

.panel.active {
  display: block;
}

    Update the JavaScript to toggle the .active class:

    if (panel.classList.contains("active")) {
    panel.classList.remove("active");
    icon.src = "./CSS/images/icon-plus.svg";
    icon.alt = "Open-FAQ-Icon";
} else {
    panel.classList.add("active");
    icon.src = "./CSS/images/icon-minus.svg";
    icon.alt = "Close-FAQ-Icon";
}
    Marked as helpful
    0
  • P
    Habeeb Kareem•470
    @olaide-hokPosted 15 days ago

    Great work here!

    Some feedbacks:

    1. Redundant Event Listener: The body click event listener is added every time the form is submitted, which can lead to multiple listeners being attached unnecessarily. This should be added only once, outside the submit event listener.
    const body = document.querySelector('body');
body.addEventListener('click', function(event) {
    if (event.target.id === 'main' || event.target.id === 'successMessage') {
        successMessage.classList.remove('active');
        form.classList.add('active');
    }
});
    1. Default Value Logic: The default value of ratingValue is set to 5, but this assumes that the user will always select a rating. If no rating is selected, the code should handle this case explicitly.
    let ratingValue = null; // Default to null or undefined
ratingOptions.forEach(option => {
    if (option.checked) {
        ratingValue = option.value;
        console.log("This is the rating selected: " + ratingValue);
    }
});

if (ratingValue === null) {
    alert("Please select a rating before submitting.");
    return; // Exit the function early
}
    0
  • P
    toshirokubota•580
    @toshirokubotaSubmitted 18 days ago
    What challenges did you encounter, and how did you overcome them?

    I found the followings challenging: converting a checkbox into a sliding switch and customizing a progress bar with different colors. I was able to find solutions via web search and consulting with an AI chatbot.

    What specific areas of your project would you like help with?

    Any suggestions and comments are highly appreciated. If you see any glitches, please let me know.

    frontend quiz app

    1
    P
    Habeeb Kareem•470
    @olaide-hokPosted 16 days ago

    Great work here!

    Some UI improvements would be adding the extra css rules

    label > .result-icon {
align-self: start;
}

body.light-theme .strong-bg:hover {
    border: 3px solid var(--purple);
}

body.light-theme progress {
  -webkit-appearance: none;
}

body.light-theme progress::-webkit-progress-bar {
  background-color: var(--white);
}

    You can check here on how to style a progress element.

    For the JS side, some areas of improvemnet would be

    1. Code Duplication: There is some duplication in the code, such as clearing and setting classes for category_areas. You can refactor repeated logic into reusable functions such as
    const resetCategoryAreas = () => {
  category_areas.forEach((area) => {
    const icon = area.querySelector('.category-icon');
    icon.className = 'category-icon'; // Reset classes
    area.querySelector('.category-name').textContent = '';
  });
};

const setCategoryAreas = (category) => {
  category_areas.forEach((area) => {
    const icon = area.querySelector('.category-icon');
    icon.className = `category-icon ${category.toLowerCase()}-icon`;
    area.querySelector('.category-name').textContent = category;
  });
};
    1. Event Listener Organization: Event listeners are scattered throughout the code, making it harder to track and maintain.You can group related event listeners together and consider using event delegation where applicable.
    // Group event listeners
const setupEventListeners = () => {
  // Form submission
  form.addEventListener('submit', handleFormSubmit);

  // Start game buttons
  document.querySelectorAll('#start-page button').forEach((button) => {
    button.addEventListener('click', () => startGame(button.textContent));
  });

  // Radio button selection
  form.addEventListener('change', (e) => {
    if (e.target.matches('input[type="radio"]')) {
      handleAnswerSelection(e.target);
    }
  });

  // Play again button
  play_again.addEventListener('click', showInitialPage);

  // Theme switch
  theme_switch.addEventListener('click', toggleTheme);
};

// Call setupEventListeners at the end of the script
setupEventListeners();
    1. Use of const and let: Some variables that don’t change are declared with let instead of const. Use const for variables that are not reassigned.
    const form = document.getElementById('form');
const startPage = document.getElementById('start-page');
const playPage = document.getElementById('play-page');
const resultPage = document.getElementById('result-page');
const body = document.querySelector('body');
    1. Theme Initialization: The theme initialization logic is verbose and could be simplified. Consider using a ternary operator for cleaner code.
    const savedTheme = localStorage.getItem('theme');
if (savedTheme) {
  body.classList.toggle('light-theme', savedTheme === 'light');
  body.classList.toggle('dark-theme', savedTheme === 'dark');
  theme_switch.checked = savedTheme === 'dark';
}

    Well done!

    Marked as helpful
    0
View more comments