Skip to content
OverviewSolutions
36
Comments
25
Carl Wicker
@carlwicker

All comments

  • Carl Wicker•1,055
    @carlwicker

    Hey ya Joako, great work on the layout, I noticed a few minor issues.

    • A user can upvote multiple times.
    • Desktop version should have a larger indent on replies, see design files.
    • Missing CreateAt field displayed on a comment.
    • Fix accessibility and HTML issues in the report.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Hey ya Priyanka, great job on the layout, looks really good. There are however a few functionality issues...

    • Mobile slideshow of product images isn't working.
    • Delete item from Cart isn't working
    • When the users adds multiple items to the cart, the quantity on the page should reset to 1.
    • You should also consider clearing all the accessibility and HTML issues in the report.

    Keep up the great work!

  • Carl Wicker•1,055
    @carlwicker

    Hey ya Washington, everything looking good.. I only have a couple of suggestions...

    • dark/light mode not implemented
    • Submit input on Enter keypress for ease of use.
    • fix those pesky accessibility and HTML issues in the report.

    Great work!

  • Carl Wicker•1,055
    @carlwicker

    Hey ya, just been checking out your solution. There are a few issues, the most important is the speed. There seems to be bottleneck somewhere, as the data is very slow to return.

    It might be worth getting the all API data once and storing it (since it doesn't change) to reduce APIs calls. Maybe that slowing it down, but haven't looked into your code.

    Also...

    • search functionality is not working from the input field.
    • consider adding a option to clear the filter on the dropdown, i know its not in the design but it makes sense to be able to clear the filter, I believe this was overlooked by the designer.
    • finally, fix those accessibility and HTML issues in the report.

    Layout is looking good, I like your tweaks to the original design.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Hey ya Nyein, Great job, I cant see any issues. There is a weird media breakpoint on the desktop version which makes stuff jump around. Personally I'd lose the additional break point.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Hey ya manik, good job on the layout. There are a couple of issues...

    • links aren't saved to local storage, shortened links disappear on page refresh.
    • mobile copy button has no left/right padding.
    • consider doing the input validation check on every change rather than submit.
    • fix html and accessibility issues in the report.

    Keep up the great work.

  • Carl Wicker•1,055
    @carlwicker

    Hey ya Dusan, all looking good however I've noticed a few small issues.

    • When resizing the page width, section text content has no padding.
    • When hitting the media query breakpoint the mobile navigation briefly appears (something to do with your transition animations I would imagine).
    • Finally, fix those accessibility errors in your solutions report.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Hey ya Saykeed, the functionality all seems to be working fine however the styling isn't quite right yet. Firstly I'd go through the report and address the issues, start by changing the ids to classes which will clear lots of your reported errors.

    Here's the link for your report if you haven't seen it: https://www.frontendmentor.io/solutions/css-and-javascript-json-xOO-pLf5Y/report

    "id" can be used on an HTML element that appears once on a page, however "class" can be applied to multiple HTML elements.

    The style guide document provides links to the relevant fonts, font sizes and colors, they don't seem to be applied to your solution so take a look at that after you've addressed the report issues.

    Keep up the great work.

  • Carl Wicker•1,055
    @carlwicker

    Really great work, consider fixing those accessibility issues in the report.

    • There should only be one H1 on a page, so probably move that to the name.
    • Completely remove all the other header tags, consider spans or divs.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Hey ya, good job so far... you're not far off getting it right.

    • firstly I'd focus on getting your panel centred on the page, you could achieve this with a few amends to the body CSS and a bit of flexbox centring and a height: 100vh.
    • the floating @box also doesn't appear in the right place when you resize the page, consider again using flex and losing the left and top property and replacing it with a negative margin-left and a positive margin top.
    • accordion FAQ breaks on mobile.

    Not sure what's going on with your git problems.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Hey man, good job on the layout however there are some issues that should be addressed.

    • the page content seems to jump about when you mouse over a product thumbnail
    • default to add to cart should 1 not 0.
    • adding additional items to the cart should be added as a new list item rather than just increasing the quantity.
    • clicking on a navigation clears the cart.

    Finally fix those accessibility and HTML issues in the report.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Great job on the layout, however there are a few other issues that I thought I'd bring up for consideration.

    • If you click on a nav item it clears the cart, I think this is caused by your hrefs reloading the page, if you going to keep them as clickable links look into react-router otherwise remove the hrefs to stop the page refresh.
    • you should reset the amount of items to add to cart once the add button is pressed to it's defaults back to one.
    • if you add additional items to the cart, I think it's better to add them as a separate list item in the cart.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Great job, there are however a few other issues I would consider fixing.

    • The input field validation doesn't always work, it should revalidate on every change.
    • And submit on Enter key press for the input field would help accessibility.
    • on clicking the submit button with incorrect data the input breaks and is no longer usable.

    Finally fix those accessibility and HTML issues in the report.

    Layout look good, great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Great job on the layout however there are a few things missing from the design specifications.

    • Add a drop shadow the country cards, this will give them a clearer outline on the light mode.
    • The search and filter does not work correctly, the search works globally but not if the country filter is applied.
    • I'd consider making the whole filter dropdown clickable to ease accessibility.
    • Bordering countries on the details page currently shows the country code, it should be the full country name. To achieve this you will need to re-query the API.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Great job on the layout however there are some issues with the search and filter.

    • If I first select "Africa" as the filter and then search using the search bar, the results are global not just for Africa.
    • The bordering countries buttons on the details page are also using the CCA3 names, it's meant to be the full country name, you'll need to re-query the API to achieve this.

    Finally fix those accessibility and HTML issues in the report.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Good job on the filtering however the details page is missing, the design files also show a details page when you click on a country.

    I would also consider making the whole filter dropdown clickable rather than just the arrow for ease of use.

    Also, fix those accessibility and HTML issues in the report.

    Keep up the great work.

  • Carl Wicker•1,055
    @carlwicker

    Great job on the layout. However, if I remember rightly the text to right of main image is meant to change too with every image. The sliding images also seem to move up by a pixel once in place, not sure what's causing it but maybe take a look.

    Also fix that pesky accessibility issue in the report.

    Keep up the great work.

    Marked as helpful
  • Carl Wicker•1,055
    @carlwicker

    Great job on the layout, however there are some issues....

    • Desktop Hero header should be left aligned.
    • Desktop Hero image should be to the right edge of the page outside the container.
    • Desktop Hero section design breaks if you resize the window width.
    • The form validation and warnings are missing.
    • Input text is not styled.
    • Links are not saved to localstorage (part of brief).
    • Background grey is incorrect colour to style guide.
    • Mobile menu styling is broken.
    • Sign up button on navbar should be capitalised not uppercase.
    • Desktop navbar needs top padding.

    Finally fix those pesky accessibility and html issues in the report.

    Keep up the great work.

    Marked as helpful