Skip to content
OverviewSolutions
7
Comments
13
cesar hernan ruscica
@hernanruscica

All comments

  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    hello AnastasiiaPushkarev. First of all, congratulations on coming back to the frontend world and welcome to this platform. The page looks great, very similar to the required design, although the vertical centering would be missing. About the code, don't forget to use the semantic tags, not only divs... the css code is also very good, I would only add that you can use "custom variables", to better control the possible different layouts. "Custom variables" would be very useful for making "night" and "day" versions. Overall the project is very well completed. Greetings and I hope you like the comment.

    Marked as helpful
    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hello Swardha Sawant, first of all, congratulations for completing the challenge, even if it is an easy one, it is important for being one of the first. Visually it is very similar to what is required. About the code, I tell you some suggestions:

    1. since it is supposed to be a component, it would have been better if the component itself was aligned with margin attribute, instead by the parent "body" container.
    2. The class name "Container" is very generic and into a larger code, it would generate class interference.
    3. I would like to make the selection of the elements in the css code, by classes name instead generic selector like "h1". But overall, it's a well-done project. Greetings and until next time
    Marked as helpful
    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hello Carlos Equiz, very good work. It's a great start, the final result was very similar to the design given.

    Surely you are going to refresh your knowledge, participating in this community is the best way.

    About the solution: -The first thing you see in the comparison is that the card sizes are different.

    • you also forgot to add the shadow, which is very subtle.
    • to make it look more like it, you should have set the background color to the body.
    • for styles, it is recommended to use classes and not the Html tags directly.
    • you should have improved the Html structure, using a "main", "section", "article", etc.

    But in general I liked it a lot, to continue working because the other challenges are more difficult. Greetings.

    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hello WhiteFatal, this solution is great, I liked it a lot. The result solution seams to be the same to the disegn. Analyzing the code, I can tell you that:

    • As you say, I think you gave too much importance to the "footer", when it was not the main thing. Besides that code should be more simple, it was not necessary that you position it in each "media query". On the other hand, if you had started "mobile first" you would have done it with a single "media query". On the other hand, it is good practice to separate the HTML from the CSS code. Overall, an excellent job, I hope you continue on the platform. I'm starting too and I think it's very good. Greetings.
    Marked as helpful
    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hi Gabrielkyalo, I was seeing your solution. It is very good, that you have encouraged to share it and participate in this community. It will help you to improve your codes.

    About this solution, I can tell you that it doesn't look very similar to the challenge design, I think you should have tried to make it more similar with all the details. For example in the "paddings", "font-sizes", colors. you even miss the "boxshadow" of the card.

    Regarding the code, you should use semantic HTML, relative units. In addition, to use less lines of code, you can use the "shorthand" of the different attributes.

    They are details, but they make the master. It is very good, that you are here, and that you have sent your solution.

    Greetings.

    Marked as helpful
    1
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hello RickGatica, I really like your solution for this challenge. I think it meets the requirements, although it obviously doesn't look the same. The sizes of the fonts are others and the sizes of the containers. I really liked the fact that you used a single image. I did the same, although some people told me that I should have used the two provided, with the new "picture" tag that lets you change the image according to the "viewport". Another suggestion is that you use semantic tags instead of simple "div". It is also recommended to use relative units. And finally, I would put the fonts and everything related to the component, inside the component, not in the body, as you did with the fonts. Beyond that, the solution is great and I like it. Keep it up, you're on the right track.

    Marked as helpful
    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hi Luis0251. I was seeing your code and I really like it.

    • The code Is well organizated and clean.
    • I think the mediaquery isn't necessary.
    • the selection of the subitems of the container class, should be made with another class. Like in BEM style.
    • should use semantic tags for the Html. Overall this is a great job, I like it very much, I made it very similar! Greetings and keep going !!
    Marked as helpful
    0
  • Harry Lee•40
    @BestNerdSubmitted about 2 years ago

    I found this okay. I found the sizing a little strange, which may have been because I was using flex box, but none the less was not too bad. My github website does not seem to include the actual image of the QR however when I run it in VS code looks almost identical.

    QR code css and html

    4
    cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hi Harry Lee, i was seeing your solution for this challenge. The first thing i would like to say is that you should check the link to the QR code, because it doesn't load. About the code, you should choose better name classes, with more meaning. I would like to see it working with the QR code, to see the sizes, but overall is done. Grettings.

    Marked as helpful
    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hi Walid! The live site doesn't work. I was seeing your code, but prefer to wait until it works! Check that!

    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hi Julien. I was seeing your solution. It hasn't any issues in the report, congrats for that. My thoughts:

    • the visual result it doesn't look equal, first of all for the sizes. It must just because you understand that 375px is the width of the card container, and it is the size of the viewport in the mobile design .
    • you should use relative units.
    • if you are creating an component, the font family hasn't to be in the body.
    • you probably should use custom variables for the colors and fonts. Overall is great, and working, great job! Greetings
    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hi Kristina, I really like your code, because it is very simple. But I think you should use relative units, like EMS. Because I think, the challenge show us how it must to look in 375px mobile, but never say it has to be fixed. Other subject is that you don't use semantic HTML. But overall is great, and another think I like is that didn't use media queries! They aren't necessary in this project! Getting!!

    Marked as helpful
    0
  • cesar hernan ruscica•320
    @hernanruscicaPosted about 2 years ago

    Hi @SrHatcher . It's great that you wants to improve yours skills, and I think this is the best place. Comments from the code:

    • You could use BEM style for the classes, since you almost did it, but with one -. BEM is with two underscores ___ separation.
    • I like the "object-fit: contain;".
    • must to use the font size that the disegn give you.
    • the 375 PX width for mobile that isn't means that the container has to be that size. It give you an idea of the size of the qr-container . But after all, great job, keep going! Greetings!
    Marked as helpful
    1