Skip to content
OverviewSolutions
13
Comments
5
P
Freddy Santana
@Creixz

All comments

  • Freddy Santana•190
    @CreixzPosted 9 months ago

    Hello my friend, congratulations for finish this challenge, i like your BEM methodology and the mobile-first design, but i have some suggestions for you:

    • It's better to use rem instead of pixels.
    • The color of the <p> and <li> are incorrect.
    • Use pseudo-elements like ::before, it could help you with your list-style
    Marked as helpful
    0
  • Benhemin•180
    @BenheminSubmitted 9 months ago
    What are you most proud of, and what would you do differently next time?

    .

    What challenges did you encounter, and how did you overcome them?

    .

    What specific areas of your project would you like help with?

    .

    Omelette Recipe Page using HTML and CSS

    2
    Freddy Santana•190
    @CreixzPosted 9 months ago

    Hello my friend, congratulations for finish this challenge, let me show you some details that can help you to improve your code.

    • It's better to use good classnames that can make your code more understandable, I use BEM like this:
    <section classname= 'card'>
    <article classname='card__title'>
        <p class='card__title-description'>Your text here.</p>
    </article>
</section>
    • Use margin-left=auto and margin-right=auto to center your card.
    • It's better to use rem instead of % or pixels.
    • Use max-width instead of width, it will look better.
    0
  • Freddy Santana•190
    @CreixzPosted about 2 years ago

    Hello my friend, Congratulations for finish the project, its seems pretty good, but I have some suggestions regarding your solution if you don't mind:

    • Use BEM methodology it's a good practice, like this:
    <div class="card">
    <div class="card__header">
        <img class="card__img" src="images/image-qr-code.png" alt="Qr-Code">
    • Put the card inside the container, like this:
    <section class="container">
    <div class="card">
    • Don't use and specific height for the img, just use width= 100%; to adjust to the container and not use unnecessary margin-top.

    Thank you for taking the time to read my feedback I hope it helps.

    Marked as helpful
    0
  • Freddy Santana•190
    @CreixzPosted about 2 years ago

    Hello my friend, Congratulations for finish the challenge, its seems pretty good, but I have some suggestions regarding your solution if you don't mind:

    • You can implement BEM's methodoly because it's a good practice and its not so difficult. Example: Instead of
    <div class="container">
    <div dclass="container">
        <img class="qrcode" src="./images/image-qr-code.png" alt="">

    use this

    <div class="container">
   <div class="card">
       <img class="qrcode" src="./images/image-qr-code.png" alt="qrcode" class: card__image>
    • Use max-width: 350px; in your card because its a mobile design and then use width: 100% in your card__image to adjust the image to the card

    Thank you for taking the time to read my feedback I hope it helps.

    Marked as helpful
    0
  • MURRAY122•280
    @MURRAY122Submitted about 2 years ago

    For the image overlay. Is there a way to change or control the fill color of a background image SVG so for example:

    background-image: url("link to some svg file");

    When I used this and lowered the opacity of the parent element, the white color of the SVG eye image also lowered in opacity. So, I tried to research and find if the SVG's fill color could be changed while being used as a background-image and found nothing. So, I would have to assume there isn't a way? In the end I ended up using a second div and image tag to hide and show when hovered on.

    Any other feedback is always appreciated, thanks for viewing

    NFT Preview Card Component

    3
    Freddy Santana•190
    @CreixzPosted about 2 years ago

    Hi Murray122, first of all contratulations for complete the challenge, I think you do a pretty well job and I have some suggestions regarding your solution if you don't mind:

    Instead of put the icon-view in your html, I think its better to put in your css code, like this:

    .card .image::before { content: url(images/icon-view.svg); position: absolute;

    I saw that you use BEM's methodology, but i think that instead of this:

    <div class="section__content"> <h1 class="section__content__title"><a href="#">Equilibrium #3429</a> </h1>

    you can use this:

    <div class="section__content"> <h1 class="section__title"><a href="#">Equilibrium #3429</a></h1>

    because it's not necessary to repeat all the name, i saw in many post that is not a good practice. Hopefully this feedback helps.

    0