Skip to content
OverviewSolutions
2
Comments
5
Phenics13
@Phenics13

All comments

  • Phenics13•60
    @Phenics13

    Hi! I have some suggestions:

    • you can make the card smaller (height: 80% - 80% of height of the parent tag) to prevent scroll. However, if your pages may be high enough to scroll, replace height: 100vh to min-heigh: 100vh. It will prevent the page to look offset.
    • Or you can go the other way: make the whole parent div (that contains <QrContainer /> and <AttributionContainer />) display: flex (that you have already done). Than set <QrContainer> root component with property flex: 1. It will stretch your card with maximum height and also contain AttributionContainer in place. Learn more about flex-grow.

    I hope it helps 😊

  • Phenics13•60
    @Phenics13

    Hi. I looked at your solution and have some suggestions.

    • to make component responsive, you use media queries and that is the only way to make component responsive. However, you can also use some properties in % value to make it stretch, max-width and max-height.
    • to make appear or disappear "Thankyou" component use useState hook in the parent component(<App/>) instead of doing it in <Ratings> component. Also learn more about different hooks in React.
    const [isSubmitted, setIsSubmitted] = useState(false)

// in the App return block:

return (
  {!showThankyou ? <Ratings/> : <Thankyou/>}
);
    • add guard function that prevents user from submitting with 0 rating (because you can not pick zero) and only submit when any value 1-5 was picked.
    if (!selected) return;
    • to prevent props drilling you can use React Context (for selected number, for example)

    Hint: you can look at my solution of this challenge. It contains everything that I have mentioned here

    Marked as helpful
  • Phenics13•60
    @Phenics13

    Hi! Took a glance and have some suggestions

    • add {cursor: pointer} CSS property to buttons
    • instead of margins for every tag you can use flex or grid display property and set gap or column-gap and row-gap. It is more convenient.
    • wrap all of your content in main tag. Learn more about semantic tags.
    <body>
    <main>...</main>
</body>
  • Phenics13•60
    @Phenics13

    Hi! I viewed your code and I have a couple of suggestions for best practices.

    • You should wrap all your content in main tag after body tag. It is for SEO purposes.
    <body>
	<main>...</main>
</body>
    • Wrap image with container with fixed dimensions or set image to fixed dimensions (width, height). It prevents page's content from "jumping" when you load site with poor internet speed. To test it, if you use Chrome, open dev tools, go to Network, and pick network type: slow 3G.
    • header tag is used to contain navigation panel of a site. It is not appropriate to use it in content of the site. Read more abour semantic tags and how to use them.
  • Phenics13•60
    @Phenics13

    Hi there! I want to give you some pieces of advice how to solve problems which I faced during this challenge too and level up your frontend skills.

    How to avoid Accessibility report warnings.

    Use semantic HTML tags to avoid warning "Document should have one main landmark". Just wrap your div with class "attribution" in main tag like so:

    <main>
  <div class="attribution">
  </div>
</main>

    How to solve difficulties with height and width.

    Before implementing a challenge in a code you can try to copy the given design in Figma. Just open Figma, place the design image in it, change the opacity of the image to 50% and just create design blocks over the image. Doing so you will be able to know height and width of some basic design elements.

    I hope, my feedback was helpful 💪

    Let's improve our frontend skills together 🎉

    Marked as helpful