Skip to content
OverviewSolutions
16
Comments
16
Adesh Katiya
@adeysh

All comments

  • P
    RF13•610
    @rf1303
    What are you most proud of, and what would you do differently next time?

    I’m most proud of how I was able to break the JavaScript into modular files to keep the logic organized and maintainable. It made handling features like the image gallery, lightbox, and menu behavior much cleaner. I also paid special attention to accessibility and responsive design, which improved the overall user experience across devices.

    If I were to do this project again, I would plan the JavaScript architecture earlier and consider using a small framework or components system for better scalability. I would also write unit tests for some of the key interactive features and focus more on advanced accessibility practices like keyboard trap handling in modals and ARIA live region updates.

    What challenges did you encounter, and how did you overcome them?

    the challenges, the lightbox and the modules and the screen changes, the cartbox items and the menus, working a lot and testing, I think it looks pretty good.

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

    I'm missing more accessibility, some tips and js part would also be appreciated.

    HTML CSS SCSS Javascript Mobile firts bem Accessibility

    #bem#accessibility
    1
    Adesh Katiya•420
    @adeysh

    Hi There! This looks awesome. You have done very well implementing all the funcitionalities of the design. Your code is well structured too. I would like to give you some feedback from my end for polishing it as well here goes:

    For UI

    • The cursor currently doesn't change to pointer on hover on header contents, the preview image of the gallery, '+'/'-' and add to cart button. Some styles are missing on hover for header cart button and profile button
    • Currently the lightbox overlay screen is not covering the whole screen and white background shows up at the bottom.
    • There's no hover styles for thumbnail images in the lightbox.
    • Changing the preview image in lightbox thorugh arrow buttons doesn't update the thumbnail images telling which image is currently active. it works when you click on the specific image but not on sliding it to a different one.
    • No hover styles for close button in lightbox.
    • For tablet screen sizes the preview image on the gallery doesn't fit 100% and gets cropped out.
    • The navigation menu in tablet screen sizes doesn't attach to the edge of the screen because i think it has some fixed width.
    • I think after adding some items and clicking on add to cart button currently updates the cart badge but it should also open the cart to show the items that were added.
    • The cart if opened should close after clicking anywhere outside it.

    For Code

    • I think in the <body> tag, there should really be only three landmarks <header> containing your navbar, <main> containing everything below it.
    • The other <divs> could just be a part of a parent container even for overlays.
    • Always try to give alt messages for images.
    • The html is semantic as well.
    • I have applied the same logic to slide the image with transition that you have used for gallery in your productSneaker() and prodSneakLight() functions. however i see that you can create a single function for these two generalizing similar code.
    • Same goes for event listeners in you main.js file as well. Since you are calculating the index for when buttons are clicked and then increasing/decreasing them. you could create a single function for this operation and pass parameters as needed just for increasing/decreasing and the element that you need event listener for. it can reduce some amount of code lines.
    • These code blocks too
    btnsThumbail.forEach((btn) => {

        btn.addEventListener("click", () => {
            const i = parseInt(btn.dataset.img);
            productSneaker(i, 1);

            btnsThumbail.forEach((b) => b.classList.remove("button__thumbails--active"));
            btn.classList.add("button__thumbails--active")
        });
    });

    and

    btnsThumbLight.forEach((btn) => {
        btn.addEventListener("click", () => {
            const i = parseInt(btn.dataset.img);
            prodSneakLight(i);
            btnsThumbLight.forEach((b) => b.classList.remove("button__thumbails--active"));
            btn.classList.add("button__thumbails--active")
        });
    });

    have similar logic so you could combine them.

    • for code in the cart box section too.
    • Code in functions in menu.js file also have a lot of similar code that can be reused or generalized into a single function. for example for lightBoxChange(), lightClose() and lightBox() could have the constants defined at a single and place and then if conditions applied for necessary code.

    That's all. Everything looks good but I think you can refactor code here to make it scalable and reduce DRY code. All the best 🎉

    Marked as helpful
  • P
    RF13•610
    @rf1303
    What are you most proud of, and what would you do differently next time?

    Me gustan los desafios , el uso de grid y las transiciones de los elementos

    What challenges did you encounter, and how did you overcome them?

    It was a challenge to ensure smooth transitions between breakpoints while keeping the structure semantic and accessible to screen readers. I also implemented custom styles for the interactive elements and managed the menu opening.

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

    **I would like to receive feedback from the accessibility side.

    HTML CSS SCSS Javascript Mobile firts bem

    #accessibility#bem#sass/scss
    1
    Adesh Katiya•420
    @adeysh

    hi there! This solution looks great. Congrats on completing it!

    Here's my feedback for you:

    CODE:

    • Try to always include alt messages if you're using img tags. It helps with accessibility and screen readers.
    • Since you're already using a semantic tag nav for the navigation bar. You don't have to include role="menu" or role="menuitem" for the list inside the nav or list-items inside the list. Screen reader will use the nav as landmark.
    • The contents inside the menu list are links that can link to other sections of the same page or on another page. So they must be wrapped in anchor a tag. Though for the design they may point to hash (#) link.
    • In the main tag wrapper, you're using sections for dividing the content but you have given a class of wrappers__article. This is kind of conflicting. You could either change the class-name or use article tags for separating the content. I would suggest using article tags.
    • Since predominantly there are only text content inside the homepage that relate to being a news-homepage, there is no need for using an aside tag here. It could simply be another section.
    • Rather than using an empty div, you could give a border-bottom to the children of the New section.
    • The headings inside the <h3> tags are links to other articles, they should be in anchor tags.
    • The content in the bottom of the page are really just blog link cards. They should not be in the footer since they are not navigation links which mostly appear in the footers.
    • Here too, you can use anchor tags inside the headings.

    UI:

    • The desktop ui looks really good. There is somewhat of horizontal scrolling which appears, you could remove that.
    • I can see a background color (gray) on each of the navigation links but could not find anything inside the inspector.
    • The hover states work perfectly fine.
    • The logo should be a link to the homepage.
    • In small screen sizes it also looks good and the overlay menu also works fine.
    • However, I can scroll the page even when the menu is open on mobile screen. It should not be scrollable since the menu opened is in focus.
    • The keyboard navigation could be fixed. Currently it does not focus on all the links on the page.
    • Same goes for mobile screens. The keyboard navigation go over the same order even when the menu is open which it should not do.

    That's all! Great going and best of luck 🤩🤞

    Marked as helpful
  • Adesh Katiya•420
    @adeysh

    Hi there! Your solution looks exactly like the design. Great implementation!

    Here's my feeback for UI and code:

    • The first name and last name field is accepting numbers as input. You could add constraints there.

    • Since you have hover effects for border on every field. I see that it is not there on the radio buttons. You could add for them as well. And on the consent checkbox too.

    • On checking the checkbox since it becomes active. I get an outline which looks unnecessary. You could remove that.

    • There is no padding on the form for mobile screen sizes.

    • On the email address field, if after putting an invalid email address i remove it and submit the form. It doesn't change the error message to 'This field is required' since the field is now empty.

    • The HTML markup looks great.

    • I am not sure why but I can use autocomplete in the text form fields even though you have used autocomplete="off" on the inputs.

    • I would suggest you to use required attribute instead of aria-required="true" since all the fields in the form are standard fields so it would be helpful for screen readers as well.

    • Also I think since all the fields are already 'required' fields. you need to refactor the if/else statements for validating them.

    • You would not need these: let isValid = isRequired ? isValidEmail : isEmpty ? true : isValidEmail; let isValid = isRequired ? isValidText : isEmpty ? true : isValidText; let isValid = isRequired ? checkBox.checked : true; and let isValid = isRequired ? checkedCount !== 2 : true; - here instead of using the checkbox unchecked count. you could simply check for a checked checkbox using

    let isAnyChecked = Array.from(radioInput).some(el => el.checked);
let isValid = isRequired ? isAnyChecked : true;

    That's all I could provide as feedback. Amazing work here that you have done! best of luck 😇🤞

    Marked as helpful
  • P
    RF13•610
    @rf1303
    What are you most proud of, and what would you do differently next time?

    Continue to make challenges and to practice and improve.

    What challenges did you encounter, and how did you overcome them?

    the opening and closing of the accord and the icon on the right side I solved it by searching for examples in freecodecamp and others on the web.

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

    **Any improvement you can say is welcome.

    HTML CSS SCSS Javascript Mobile firts bem

    #bem#sass/scss
    1
    Adesh Katiya•420
    @adeysh

    Hi There! Your solution visually looks very close to the design. congrats for that. It's keyboard accessible too.

    Looking at your code:

    • Your html is semantic. Also good use of <details> and <summary> tag.
    • Its responsive at the defined screen sizes but breaks at some points in between.
    • I see that you have made a lot of mixins but have not used them. You can remove the unused items because i can see only one mixin called tgSize that you have used.

    Nice going!!

    Marked as helpful
  • Sergio Eduardo Castro Ceballos•350
    @SergioCasCeb
    What are you most proud of, and what would you do differently next time?

    I think what I'm most proud of is that I tackled this little challenge using Angular and TypeScript, technologies I'm still getting used to. I'm also really happy with how I approached accessibility: I made sure to include ARIA labels, proper keyboard interactions, and gave users the option to go back either by clicking or just waiting, which adds an extra layer of accessibility.

    What challenges did you encounter, and how did you overcome them?

    Nothing in particular, it was a nice quick challenge.

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

    Nothing in particular, but constructive criticism is always appreciated. :D

    Responsive Interactive Rating Component

    #accessibility#angular#bem#sass/scss#typescript
    1
    Adesh Katiya•420
    @adeysh

    Hi, Sergio, This looks great!

    Although I am not familiar with js frameworks. I can still see you have applied full accessibility requirements.

    Here's what I noticed:

    • The other radio buttons are not tabable except for the first one.
    • There's no error message that is shown to the user if form is submitted empty (without selecting any option)

    Just this much. Best of luck for your future projects 🤞

  • Adesh Katiya•420
    @adeysh

    Hey there! kudos for working out building the app. It looks good and very close to the provided design. However I don't think the app is currently working since it is not changing the text on the app.

    • Possible reason being your usage of template literals in the code :
    if (totalBill !== 0 && tip !== 0 && numberOfPeople !== 0){
    tipAmountPerPerson.innerText = `$${tipPerPerson}`;
    totalAmountPerPerson.innerText = `$${totalPerPerson}`;
    resetBtn.disabled = false;
  } else {
    tipAmountPerPerson.innerText = `$${0}`;
    totalAmountPerPerson.innerText = `$${0}`;
  }

    you're using $$ here!

    • here too:
    resetBtn.addEventListener('click', () => {
  totalBillInput.value = '';
  removeBtnActive();
  totalPeopleInput.value = '';
  tipAmountPerPerson.innerText = `$${0}`;
  totalAmountPerPerson.innerText = `$${0}`;
});

document.addEventListener('DOMContentLoaded', () => {
  tipAmountPerPerson.innerText = `$${tipPerPerson}`;
  totalAmountPerPerson.innerText = `$${totalPerPerson}`;

});

    your input validation looks good otherwise. Nice working! another thing would be if you're using keypress events when you want the user to enter any values you should tell it to them with a note. That would be good UX.

    Marked as helpful
  • P
    Anamay•470
    @anamaydev
    What are you most proud of, and what would you do differently next time?

    I'm proud of the simple animation I created using GSAP. It was a great learning experience, and I’m happy with how it turned out.

    What challenges did you encounter, and how did you overcome them?

    I faced a challenge when I needed to access fetched data in another function without making it public. I struggled a bit, but then I learned about closures, which made it easy to implement.

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

    I would really appreciate some help with improving my JavaScript code.

    Time tracking dashboard with GSAP Animation

    #gsap#bem
    1
    Adesh Katiya•420
    @adeysh

    Hey there! Your solution looks really great. I love the animation 😄.

    After looking at your code and observing the ui I can tell you these points:

    • Your profile nav buttons are extending their full width inside the container that is why I can click around the empty area beside the buttons and they are clickable. I think this should work only when the button specifically is clicked.

    • The buttons show only the active state but I think the designs also included a hover state for them which is not present.

    • The three dot menu icons should change the cursor to pointer when hovered over them.

    • You could also add a transition/animation when the values are changed on button clicks.

    • I think in the range of screen sizes 768px - 1024px the activity cards should have more padding around them and the current hours font size could be reduced.

    • Although it is very ignorant but I can see the background colors for the individual cards at all the corners of the card. It seems they are somewhat peeking through. You could fix that.

    • On changing the timeframe options since you have not provided a minimum width for the cards they are adjusting themselves according to the content in them and that is why it is either shrinking it or expanding when clicked on 'daily' or 'monthly' for example in the 768px - 1024px range.

    • It looks great on mobile screen sizes.👌

    • The bem class structure looks perfect.

    • Your text-presets also look really utilized.

    • the css looks great

    • for js I think you could add two loops for getting all the buttons in the profile/user card and all the activity cards in the dashboard rather than selecting them individually thorugh id's.

    • you have used json keys very good. but I think you could create an object of all the activites and then match them with the requested timeframe. That will remove the unnecessary flow of conditionals and change the data instantly.

    • since this code workCurrent.innerText = `${activityTimeFrame[requestedTimeFrame].current}hrs`; and this code workPrevious.innerText = `Yesterday - ${activityTimeFrame[requestedTimeFrame].previous}hrs`; is written a lot of times and they are doing the same thing for every other card you could create a specific function for this. call the functions according to the conditions satisfied that would make it somewhat modular.

    • You can rather listen to a single event and then check out if it was for daily, weekly or monthly timeframe rather than listening for all the button click events individually.

    These were my suggestions. I also learned how you used gsap for this looking at your code. so thanks for that 😇. Anyway keep learning and growing. Good luck!

  • Tel•220
    @telsabate-hub
    What are you most proud of, and what would you do differently next time?

    I'm proud of completing the requirements for the JS part.

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

    I would very much appreciate any suggestions that could improve my skills.

    Responsive Newsletter Sign-up Form Using SCSS

    #sass/scss
    1
    Adesh Katiya•420
    @adeysh

    Hi There. Your solution looks great. However here are some changes I could suggest looking at your code:

    • Your main component is a card rather than form so you could change the classes for that.

    • Try to use picture tag for changing image based on screen sizes.

    • The form should not involve any other content other than form fields. For other content you could make it card specific.

    • You don't have to use span inside an li tag.

    • The html is semantic but could use some restructuring.

    • Try to use reset styles to help you with initial css styling.

    • Try to use rem and em units for sizing.

    • The component is not responsive according to the screen sizes. I think you need to look at them again. The containers overflow out of the screen on almost all screen sizes.

    These were my suggestions for what I could infer. Good luck and keep learning!

  • denise•530
    @moncadad
    What are you most proud of, and what would you do differently next time?

    Most proud my use of class list toggling along with the styling it triggered. Took me a bit working out the kinks, overall I feel like I did a good job.

    What challenges did you encounter, and how did you overcome them?

    Big challenge was getting the share bubble to position where I wanted it to due to added spacing that I wasn't 100% sure off. I used negative values on any added margin. I'm not sure if that's the norm, but it got the job done. I'd love to hear about any tips and tricks y'all may have used.

    Article Preview Component

    #bem
    1
    Adesh Katiya•420
    @adeysh

    Hey there! I see you did a good job replicating the design with good html structure. However here's what I would suggest 😇 (I have provided you feedback just scrolling through your code sequentially top-down):

    • I think you have added the scipt link for font-awesome icons 2 times by accident. You can remove it.

    • wrap the card in a <article> tag rather than <main> because it can be used independently anywhere.

    • Use <picture> tag to change the picture used in the card at different screen sizes. I think you have used a single picture so you can style it with object-position property for mobile screens. Look at the mobile designs closely you will find that they are kind of cropped from top and bottom.

    • Also you have used an img tag inside your card__img div but you have given a background image to your card__img div which is redundant and not useful. Use one way to render your image and be consistent.

    • For the other end of the card i don't think aside is semantically correct to be used here. It's just the content for the card that is typically used in any card layout so rather you can just wrap it in a div with card__content class or a section to be more semantic.

    • Inside your <article> since you have used <wrapper> to style it with padding you can remove and put styles for it rather directly in the article tag.

    • Since you're using bem naming convention, try to use bem classes for your components with classes and use only those classes to style it. Also try to divide them mindfully looking for any repeating styles that you can create a class which can act as utility like font-styles and flex-box styles.

    • Never use footer inside the main tag. Since this is a card component the footer is not particularly required as such but even if you want to use it you can do inside your article tag and with a specifying class like card__footer to show that this footer belongs only to the card only.

    • Inside your footer section, the card author and date could be wrapped inside a more meaning-full class like card__profile or card__profile-container. Also they are more like block elements rather than sections since they only have a single line of text.

    • The share section and share button is good as is. Good job

    • On click of the share button the card author text and date get shifted somewhat to the right you can fix it.

    • Try to be consistent with font-sizes preferable using rem units in your css.

    • Use your variables you have defined for font-sizes.

    • Rather than using separate class for flex-styles or disply:none you can create a separate class and add it to whatever tag you want.

    This may seem a very verbose feedback but try to refactor your code and make it DRY. Otherwise that's a really good looking card 🤩🤞

    Marked as helpful
  • P
    Jayco•470
    @jayco01
    What are you most proud of, and what would you do differently next time?

    I’m proud of how I successfully implemented CSS Grid to create a structured and fully responsive layout. Ensuring that the testimonials aligned correctly across different screen sizes was a challenge, but refining the grid-template-areas and spacing improved the overall design. I also liked how the box shadows and background images enhanced the visual appeal.

    Next time, I would focus more on accessibility improvements, such as adding ARIA labels and better contrast for readability. Additionally, I’d explore adding subtle animations or hover effects to enhance user interaction.

    What challenges did you encounter, and how did you overcome them?

    One of the biggest challenges I encountered was aligning the testimonials properly using CSS Grid while ensuring even spacing across different screen sizes. Initially, some elements weren’t positioned as expected, leading to inconsistencies in the layout. To overcome this, I refined my grid-template-areas and explicitly defined grid-template-columns instead of relying on automatic placement. Adding gap between grid items also helped maintain consistent spacing.

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

    I would love feedback on my CSS Grid implementation, particularly in ensuring that the testimonials are evenly spaced and well-aligned across different screen sizes. While I used grid-template-areas to structure the layout, I wonder if there’s a more efficient way to achieve the same result or improve its responsiveness further.

    Additionally, I’d appreciate any insights on background image placement. I used background-position and background-size to position a decorative image within one of the testimonials, but I’m curious if there’s a better approach to ensure consistent positioning across all screen sizes.

    Responsive Testimonials Grid Section using CSS Grid

    #bem
    1
    Adesh Katiya•420
    @adeysh

    Hey @jayco01! Your page looks visually stunning and very accurate to the design as well.

    The code is semantic as well and I also like how you have used <picture> tag to wrap the images and <strong> tag to show bold text for the screen readers. I realize I should use this as well 😅. It looks good on all screen sizes and readable.

    • I would suggest making the wrapper testimonial__grid a section inside the <main> tag since it can contain other types of content as well besides grid section.
    • Your naming for the bem classes for individual cards are acccording to the authors (testimonial__clifford) rather this, considering bem conventions you could use a testimonial-card or card as block and include the styles for individual authors as modifiers like (testimonial-card--clifford). You can use all the styles for an individual card here also removing the class for colours, padding and font-styles like testimonial--white.
    • Update the properties for font-sizes, font-family and box-shadow to use a custom variable since you have used it multiple times.

    Otherwise this looks awesome 😇💫

    Marked as helpful
  • P
    Gabe•320
    @gabei
    What are you most proud of, and what would you do differently next time?

    I am most proud of the outline animation that I added for desktop users, as well as the ability to tab to the different cards. It's not entirely necessary, but it was a fun challenge to try and make it work the way I imagined.

    What challenges did you encounter, and how did you overcome them?

    I am not experience with grid at all. I am more of a flexbox user, so I had to break out of the mold a bit to figure out how to arrange things properly. I can see the power in the grid, and I know that I've only scratched the surface.

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

    I would like some critique on my usage of grid. I am pretty new to css grid and I haven't delved into best practices yet.

    Four Card Feature Section

    #sass/scss#node
    1
    Adesh Katiya•420
    @adeysh

    Hi @gabe, I saw your code and I think your html markup with bem classes is pretty good. The animation is amazing! I also like how you have made the page responsive using simple code. I think that the code for the grid layout could be simplified by using grid-template-areas since you're adjusting the card length to different sizes according to the screen sizes but it will be really easy to use grid-template-areas in your code. Take a look at it and you can try to refactor and clean the code. Other-wise It looks really good and sharp. Good job 🎉

    Marked as helpful
  • Adesh Katiya•420
    @adeysh

    Hey congrats on achieving the exact design. You did a really good job! Your HTML document structure is simple yet achieves the intended content. I saw your sass files and it looks really good how you have used grid and flex properties to make spacing perfect. Especially this flex: 1 1 auto. I think I also need to use this more 😅. Anyway it looks really good on desktop as well as on mobile!

  • Adesh Katiya•420
    @adeysh

    Hey there! Congratulations on completing the challenge! I saw the code and it looks good how you have structured the whole page. Although I'm not familiar with scss right now I can see that it's readable from the class-names that you have used for the bem.

    I think the whole card could be a zoomed in so that it looks similar to the design picture. It looks good on mobile screen size as well. Good job !

    Marked as helpful
  • P
    Denisse Joyce•290
    @denissejoyce
    What are you most proud of, and what would you do differently next time?

    I like the cute little gradient animation I added for the bg 🙈

    What challenges did you encounter, and how did you overcome them?

    The animation initially was not showing when using Safari and the buttons looked weird. Not sure how I resolved the animation but with the buttons, I had to remove the backdrop filter.

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

    Any review of my code would be helpful! I'm still trying to get used to using BEM so I'd be grateful for any comments/tips about that.

    Social links profile main (gradient bg, day/night mode available)

    #accessibility#sass/scss#bem
    1
    Adesh Katiya•420
    @adeysh

    Hey congratulations on completing the design 💫💫. It looks really good with day and night color palettes. Your BEM structure looks fine to me and it also looks good on all screen sizes. All though I am not familiar with SCSS yet your code looks moduler which really helps at the end of the day 😇.

  • Adesh Katiya•420
    @adeysh

    The code is well structured and readable but I think the spacing between elements is not according to the design that is why it seems that it is a bit small. That's all

  • Vitor Souza•240
    @souza-vitor
    What are you most proud of, and what would you do differently next time?

    I didn't have any struggle to finish it this time and made it very fast.

    What challenges did you encounter, and how did you overcome them?

    I was pretty comfortable doing the challenge.

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

    I would appreciate any feedback on whether I am using the BEM methodology correctly.

    qr-code-component

    #bem
    1
    Adesh Katiya•420
    @adeysh

    • The code includes semantic HTML but you can put the card image and card text in their own containers according to the design file.

    • Layout looks good on all screen sizes but I don't know why the solution screenshot above shows some white space at the bottom of the screen. Not sure about that.

    • Code is well structured and readable but if you are using bem methodology then I think we use '__' for separating the block and elements which are missing here. Other wise its good.