Skip to content
OverviewSolutions
9
Comments
25
Shubham Sharma
@shubhamthedev

All comments

  • Shubham Sharma•855
    @shubhamthedev

    Hi the solution looks great and it's responsive here are a few things that you might wanna look into :

    • The loading takes unusually, I really don't know why this is but the api isn't that slow in sending data.

    • Search work properly only when the region is set to all regions.

    • Borders on the details page get messed up sometimes, where i can't even view countries that do have borders.

    • Dark mode doesn't work for me even though i can see the buttons.

    I did this same challenge a while back maybe it might help you, here is the link.

    Hope this helps.

  • Shubham Sharma•855
    @shubhamthedev

    Hi, I can't really see your code maybe you entered the wrong link there but what you could do is put the whole section into a <div> and give it a class .container then specify the width of the container using CSS i.e. width: 800px or whatever value you wanna give it then use position : absolute to centre the element also you can use flexbox to centre the elements.

  • Shubham Sharma•855
    @shubhamthedev

    Hi, the solution looks great and i love the theme switcher especially, here is a few things you can tweak:

    • The region filter doesn't let me switch back to viewing all countries, maybe add an option for that.

    • If i click on the border for a given country then their is no way for me to see go back an see all countries since it only shows me the country that i clicked on.

    • When clicking on the borders the page should automatically show me it's details.

    • When i click on a random country in the beginning and start scrolling on the details page i can see the main page scrolling for some reason.

    These are the things that i noticed while looking at the site, I did this same solution a while back you can checkout my solution here.

    Hope this helps.

  • Shubham Sharma•855
    @shubhamthedev

    Hey, good job on the project here are a few things maybe you could fix:

    • The borders on the details page are supposed to take you to that page, basically they are supposed to be links.

    • Okay there are way to many html and accessibility issues maybe fix those.

    I did the same project using react, you can check it out here.

    Hope this helps.

  • Shubham Sharma•855
    @shubhamthedev

    Hi, functionality wise the site works but their are a few problems :

    • The filtering using filter region doesn't work you actually need to make a new call to the api to filter by region.

    • On the details page you cannot see the surrounding borders for a country .

    • Plus you have a lot of html and accessibility issues.

    But i know this isn't an easy challenge so overall good job. Here is my solution for the same.

  • Shubham Sharma•855
    @shubhamthedev

    Hi, I don't see any problems with the design or the functionality, the code is clean and the site is responsive, good job 👍.

  • Shubham Sharma•855
    @shubhamthedev

    Hi, awesome job on the project here's what i think you can improve:

    • First of all the icons in the search and filter regions aren't visible for some reason.

    • Second when you toggle dark mode on the homepage, it persists only until i click on the neighbouring borders of a specified country i.e. if i clicked on India the dark mode persists but as soon as i click on one of it's borders it disappears.

    • Third you are loading some wrong pages like for ex. when i clicked on India it opened up British Indian Ocean territories page, maybe you're making the wrong api requests.

    Here is mine solution although i did it using React.

    Hope this helps 😊

  • Shubham Sharma•855
    @shubhamthedev

    Hi, the loading skeleton looks great in fact i didn't know such a tool existed so thanks i learned something from you and on the subject of theme switching, it seems to work fine. But here's how i did mine, basically when i toggled dark mode i basically stored this info in local storage plus i made sure that as soon as body gets a class of .class everything else on the page also changes.

    This is my Solution

    Hope this helps!

  • Shubham Sharma•855
    @shubhamthedev

    Hi, here are some of the answers to your questions:

    • For me i usually change the font sizes for the paragraphs on different screen sizes and in combination with width attribute it seems to work fine for me.

    • I'll be honest but i have never used flex-wrap ever but you could see a small example of how it maybe useful over here.

    • You could use a shorthand background: #ffffff url("img_tree.png") no-repeat right top; and put both the background-image and background-color on the body element,this worked for me.

    Hope this helps.

  • Shubham Sharma•855
    @shubhamthedev

    Hi @LizUK, nice job and you've nailed the line heights and font sizes pretty accurately and you've got the positioning done correctly. Here are some suggestions for you:

    • You should provide a label for your inputs since labels are used by the accessibility API for screen readers and other things.

    • If you're asking for the users email address then the input type should be email i.e. <input type="email"> and not text.

    • Plus you should use a form submit button or in other words the button element instead of an input element.

    • I don't know the reason but on my screen width the design has a horizontal scrollbar which should be there.

    Hope this helps and keep coding 😃

  • Shubham Sharma•855
    @shubhamthedev

    It's a simple concept if you designed your site first for desktop screen sizes like 1400px then you need to scale down your design using max-width media queries and if you designed your webpage for mobile first screen sizes like 375px then you use min-width media queries to scale up your design.

    Here is an MDN page for the same although this contains a lot of details and might confuse you so here is a youtube video for the same.

  • Shubham Sharma•855
    @shubhamthedev

    Hi, you could use display:inline-block to put them on each other sides, or if you put them inside a <div> element then you could use display:flex property on the <div> element to display them on each others side. Apart from this your design seems pretty good the design is responsive and adapts nicely to different screen sizes.

    • Just a few suggestion, the button at the bottom of the page needs more space to fit the text maybe increase the width of the same a bit plus you should provide some transitions to the way buttons change color upon hovering.

    • Second fix all those html validation and accessibility issues.

    Hope this helps and keep coding 👨‍💻.

  • Shubham Sharma•855
    @shubhamthedev

    Hi @ntchungse, you could stack all the card on top of each other when you reach around 900px. Plus you should decrease the margins on the main heading at the top. Other than that the design seems fine.

    Hope this helps and keep coding 😊.

  • Shubham Sharma•855
    @shubhamthedev

    Hi @predeyo, i think there is something wrong with your media queries since even on a laptop screen it seem to be in a tablet portrait form.

  • Shubham Sharma•855
    @shubhamthedev

    Hi @apoorva2019, you should keep the <footer> outside the container since that limits the size to which the background-color can spread. You can put the contents inside the footer in a container though i.e.

    <footer>
   <div class= container>
      your content goes here
   </div>
</footer>
  • Shubham Sharma•855
    @shubhamthedev

    Hi Matas, nice work on the design, it is responsive and looks good on most view-port sizes. Here some suggestions for you:

    • You should probably give some margin-right on the social icons they are sticking really close to each other.

    • You should give a cursor:pointer to your button element.

    • Always provide altattribute to your images since they're important for screen readers and for general accessibility purposes.

    • Input should always have a label with them and can be hidden in CSS.

    • The input type should be email since you are taking the email address from the user not text and this provides some basic HTML5 validation.

    Overall nice job and keep coding 😊

  • Shubham Sharma•855
    @shubhamthedev

    Hi @ornel77, your design is looking great and here are some fixes for your issues:

    • You should provide a label for your input. You can hide it in CSS using display:none property.

    • The <form> element shouldn't have an empty action attribute just remove the attribute all together.

    • The input type should be email rather than text since users would be entering an email address and html5 provides some basic form validation when type is set to email.

    Hope this helps and happy coding.

  • Shubham Sharma•855
    @shubhamthedev

    Hi, if you're using flex-box for this project then i don't think there is any other way to position logo on top other than to use position:absolute. The site is responsive although i have some suggestions for you:

    • You should fix all the accessibility issues by providing a label for input and name attribute for button element.

    • The site is responsive on tablet devices although the image is very stretched and doesn't look good.

    • You forgot to add the background to the site.

    Hope this helps and keep coding 👨‍💻