Skip to content
OverviewSolutions
20
Comments
30
Shiva
@shivaprakash-sudo

All comments

  • Shiva•670
    @shivaprakash-sudo

    Hello sagekim6,

    I've tried to check the live site, but it seems like the styles and the images are not loading on to the site.

    • This could be due to path issues when you're trying to link them into the html file.
    • You need to convert the scss to css and then link the CSS file, not scss file.
    • Why are you using js as file extensions when using react?
    • Try to start with mobile screens first and then go for larger screens. This way there are no need of many media queries.
    • Also, try to use min-width in media queries, instead of max-width.

    Good luck!

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Jordi Mantilla,

    The chart component itself is looking good, but unfortunately I'm not able to see the generated chart.

    I just checked in Edge browser and it is miraculously working!😂 I think the problem was with the Firefox browser, because I get the following error in my console, when I checked for any errors. Uncaught SyntaxError: import assertions are not currently supported. So lookout for these kind of things and try to do cross-browser testing.

    Anyway, after checking the bar chart in Edge, I have few suggestions.

    • The bars could use some width in bigger screens, they look too skinny on my laptop screen😅.
    • The hover states are working fine, but all of them are at the same height, which in my opinion, doesn't make sense in user experience wise. Also, in the design they're just above the bar, so try to achieve that. This can be done by putting the money positioned absolute to it's respective bar.
    • Try to update your readme file using the readme template given along with the project files. It helps when someone else is looking at your repository.
    • Try to wrap the card using more semantic tags like section, article etc., and the attribution using footer.

    Keep up the good work!😊

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Chakiri,

    Your solution looks perfect!

    Just a few things I'd like to suggest, if you don't mind.

    • I think you forgot to add the hover eye effect to the main image, so please look into that.
    • Try to use a separate file for styling, because it's better to maintain when the code gets bigger.
    • Try to wrap the content using main, instead of just div.
    • Usually sections are used for content which span across the full width of the screen, so here you can use divs inside the card and you can use section or article for wrapping the whole card.
    • Try to update your readme file from the README-template file given along with project files, it helps you think about what you learnt and what you can improve on further.

    Keep up the good work, I hope to see more of your work.😁

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Cellmix,

    The card looks fantastic on both mobile and desktop. I just have a few suggestions, if you don't mind.

    • A little bit of top and bottom padding to the button on bigger screens would make it look even better.
    • Try to put the attribution outside of the card and inside a footer.
    • The font sizes can be increased a little bit on bigger screens, so that they don't look small.
    • h1 tag is usually used for page headings and for other sections and components, usually h2-h6 tags are preferred.
    • Semantic tags can be used for the card component, like section, article, etc., for better accessibility.

    I hope my feedback is helpful. Good luck!

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Paula,

    The chart component looks good on desktop screen, but there are few issues with it as given below.

    • The chart doesn't seem to be very responsive, because on small screens the chart width is becoming too small to hold the bar chart.
    • I see that you're using a chart library to draw the chart, I tried using chart.js, but I couldn't make it work and look like the design, since I had no experience with it, so I dropped that and drew the chart from scratch using inline styling. The only suggestion I can provide here is that, if you're not experienced with the library, try not to use it and try to solve the problem in a different way. This way you'll get to learn many new things and also how to approach a problem without using libraries.
    • Regarding the usage of semantic tags, you can wrap the chart component inside a main tag and the attribution inside a footer tag.
    • Coming to the script, you can put JS code in a different file and link to it in the head section of the html file, this way the code looks more organized and easy to maintain.

    I hope the above points are helpful and I hope to see more of your work, good luck😊.

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Iryna,

    • You can get around the caching by putting the below code as a second argument in the fetch API.
    {
    cache: "no-cache"
}

    After I've done this in my code, I started to get a new advice every time I clicked. Let me know if it doesn't work.

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello BubbleCrown,

    The output looks amazing, even the dark and light modes are working fantastically, good work!

    Few things to consider:

    • Try to change the default README from Create React App to project specific file to showcase your work and what you've learnt. A template for this can be found in the files given with the project.
    • Why was type used as the folder name when you've created interfaces inside it?🤔
    • Also, try to check your report for the site, it seems to have many issues regarding the semantic approach.

    Happy coding!

  • Shiva•670
    @shivaprakash-sudo

    Hello Roy,

    • The styles aren't applying to your site, this is because you've put the link tags inside the body, instead of head section.
    • Try to change the default title of the document to the respective project or just use the template given in the project files.
    • Try to have only ONE html file, instead of having multiple of them in multiple folders.
    • Try not to use uppercase words for file naming, they look aggressive.
    • Try to use main tag for main content inside the body for more semantic and accessible approach.

    Happy coding!

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Tammy,

    I have few things to say:

    • Try to put the CSS in a separate file, instead of putting inside the html file.
    • It's better to create new repository for a new project, instead of having all the projects in one repository. Having own repository for a particular project is easy to handle and maintain.
    • Regarding the website, it only changes the layouts when the screen width is below 375px. This is because max-width property was used instead of min-width in the media query. The width properties given in the design are just for reference, you don't need to code particularly for those widths.
    • Try to code for mobile screens first and then change the layouts accordingly for bigger screens using min-width property in media queries. For example, have one kind of layout until 600px (mobile screens) and then use min-width: 600px (tablet screens) property to change layout for screens starting with width 600px. For even bigger screens, you can use min-width of 1000 or 1100px. Try not to put too many media queries.
    • Finally try to wrap the main content of the site inside body in a main tag.

    Happy coding!

  • Shiva•670
    @shivaprakash-sudo

    Hello Dubey,

    GitHub looks for images using relative path, so we have to give relative path to the images instead of absolute paths. Instead of the path you've given, try ./images/your-image-name.png. Here . represents current folder (one dot means one step upwards into the folder structure), since the images are inside the images folder, you go into that to choose the image.

    Few notes to consider:

    • Try doing the design for mobile screens first and then change the layouts for bigger screens using media queries.
    • Once we code for the mobile, we don't have to repeat the same code for bigger screens too, we just change what is needed to be changed after a certain breakpoint.
    • Try not use fixed widths for images or image containers, try using max-width: 100%; for images, so that they behave responsively.
    • Try to use rem or em instead of pixels, to make the site easier for responsiveness.

    Happy coding

  • Shiva•670
    @shivaprakash-sudo

    Hello Omar,

    I've checked the live site and it looks really good. I like the way you used TypeScript in the components, keep up the work!

    Just a small note, you could add few screenshots to the README file, so that whoever comes to that repo would know how your site would look without them having to open the live link.

    Happy coding!

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Archis,

    Your site looks good and everything works as expected, good work.

    Few things to note:

    • There are some accessibility issues, so check your report once and try to resolve those.
    • Try to have the same font-family as the design, so that it looks very close to the design. Keeping the site as close as possible to the design is a good skill.
    • Also, try to change the title the of the page from the default title.
    • I wanted to check the code, but the link isn't working, please check that once.

    P.S. I checked your movie app and the animation of the movie details on hover is a good touch.

    Happy coding!

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Mohammad,

    • The images are not loading on the live site, I think you need to change the relative path of the images in the html file.
    • Styles for the heading and button need to be changed according to the design (big bold heading and rounded corners for the button).
    • No need to use Bootstrap if you're going to manually style the elements or vice versa.
    • Try to put the image inside the header into a nav element.

    Happy coding!

  • Shiva•670
    @shivaprakash-sudo

    Hello Hadjer,

    • There's a style-guide.md file in every project on this site. In that there would be different type of styling guides for the project like font size, colours, font family, etc. Try checking out that first for the base styles.

    Regarding padding and margin, we can't say exactly what the value would be, from a design, but we can do trial and error to see what value is the closest one which matches the design.

    • Try playing with different CSS values for particular property like padding or margin and see how it effects the overall design. Try doing this with various properties and you'll start to get how these things work and how to approach a design.

    • For small projects like this, we don't need much organisation of files, but as projects become bigger, try researching which type of project structure would be good for the tech stack you choose.

    • Try to research about the usage of semantic tags to make your site more accessible.

    Keep up the work! Aim higher and try to reach for it.

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Thih_Nezzy,

    The Advice API caches the data for 2 seconds, which is why you're having delays in responses. You can avoid the caching using the following code, as a second argument to the fetch method. { cache: "no-cache" }

    Let me know if it doesn't solve the problem.

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Sindhuja,

    The issue would most probably be related to the path of the image you're providing in the HTML. We have to provide relative paths to the image src attribute, for example, let's consider a project structure as follows.

    QR code component
        images
               qr-image.png
        index.html
        styles.css

    In this case, the image src inside your HTML should have something like ./images/qr-image.png, where one . represents that we're searching something inside the root folder, which in our case is the images folder and inside that we're taking the image.

    I used to have this problem too, but after some researching the above solution was what I found to be helpful. I hope it solves your problem, please let me know if it doesn't.

    Also, the GitHub repository link doesn't seem to work, can you please provide a link to your source code?

    Marked as helpful
  • Shiva•670
    @shivaprakash-sudo

    Hello Abbas,

    For some reason the app doesn't work for me, I tried it in different browsers, but it doesn't work in them too. It just says loading. Could you please look into this?

    Also, the search bar could use some padding to give some breathing space for the content inside.

  • Shiva•670
    @shivaprakash-sudo

    Hello Sakshi,

    Congrats on submitting your first challenge!

    The card looks very good and even the code looks neat enough, the only things which need to be changed are:

    • Try to wrap the card inside a main tag, for more semantic approach.
    • You can put the screenshots in the README-template.md and update that accordingly, instead of updating the default README file.

    Regarding flexbox and grid, you don't need any of them for this project, they only come into picture when there are multiple rows and columns involved.

    Happy coding and good luck to your next projects!

    Marked as helpful