Skip to content
OverviewSolutions
6
Comments
5
Chris
@chrizgx

All comments

  • Chris•180
    @chrizgxPosted almost 2 years ago

    Hey! I really like your design. I also took a look at your code, and I was amazed by how it's possible to edit opacity through HSL colors directly. When I wanted to apply opacity to the color itself (instead of the whole element), I had to to find the equovalent RGBA code. Thanks for that!

    As for feedback, the only thing I can say is you forgot to add border-radius: 10px to the overlay.

    I really like the outcome, great work!

    Marked as helpful
    0
  • Jhan Galvis•80
    @AltairCGSSubmitted almost 2 years ago

    If someone can help me so that the "view" image (the eye of the image that appears when the cursor is positioned over the image) does not lose opacity when applying the hover to the image, I would greatly appreciate it.

    <div class="container">
        <img src="images/image-equilibrium.jpg" alt="NFT" class="image" />
        <div class="overlay">
          <div class="text">
            <img src="images/icon-view.svg" alt="" />
          </div>
        </div>
</div>

    I am open to feedback.

    NFT preview card component FLEXBOX | GRID

    2
    Chris•180
    @chrizgxPosted almost 2 years ago

    Hey! I think there is a much simpler solution to your problem with just 2 lines of code. The reason why opacity is being applied to the eye icon is that you apply opacity on the whole div.overlay, which includes the eye image. What you can do instead, is simply replace the HSL(178, 100%, 50%) background color with the equivalent RGBA #00fff760. It's the same color, except that opacity is applied to the background-color itself and not the whole div element.

    After that, change .overlay:hover to {opacity: 1}

    O had the exact same problem, that's the solution I used too. You can see the outcome here: https://chrizgx.github.io/frontendmentor-nft-preview-card-component-main/

    And the code too: https://github.com/chrizgx/frontendmentor-nft-preview-card-component-main/

    Feel free to ask anything, I'm happy to help

    Marked as helpful
    1
  • Chris•180
    @chrizgxPosted almost 2 years ago

    Hello, and congrats on submitting your first project! I took a look at your code, and there's my feedback for you.

    1] In general, I would recommend you study CSS flexbox. It is the most efficient way to position items throughout the whole page. (take a look here: https://css-tricks.com/snippets/css/a-guide-to-flexbox/). After you understand the concept, you can take a look at my solution too (where I use flexbox). Here is my link to my github repo: https://github.com/chrizgx/frontendmentor-product-preview-card-component/

    2] I can see you spread the title 'gabrielle essence eau . . .' by using 3 header elements. Instead, you can use just 1, and the text will wrap automatically.

    3] Using a table to position the deleted price right next to the current price is smart. However, it is not recommended to include headers items inside a table. You don't need to worry, though. Once you learn flexbox, you can recreate it without using a table.

    4] As for the button, the html part is correct. About CSS, I recommend always setting 'border: none;' for buttons, because browsers apply a border by default (which I personally don't like).

    If you want my opinion, the most critical thing to focus on is css flexbox. It is simpler than it might seem in the beginning, and it is very useful. Have fun coding and exploring new stuff. Feel free to ask questions, I am happy to help.

    Marked as helpful
    1
  • Hsienz•550
    @HsienzSubmitted almost 2 years ago

    This is my first time to use js. I want to ask if there are some code or coding style need to improve. And should I use <input type="radio> instead of <button>?

    CSS flex

    1
    Chris•180
    @chrizgxPosted almost 2 years ago

    Hello dear friend developer. Your design is great, so I'll give you some advice to make it even greater.

    1. It seems like the rating buttons div has less width than the submit button. That is happening due to the margin you added to .ratingBtn class. To fix that, you have to replace:

    .ratingBtn { margin: 10px;} which applies 10px margin on EVERY side (top-right-bottom-left) with .ratingBtn { margin: 10px 0px; } which applies 10px margin ONLY on top-bottom, and 0px margin on left-right.

    Then, you'll have to change the #ratingBtnDiv from { justify-content: space-around ; } to { justify-content: space-between; } in order for the buttons to be equally positioned throughout the whole width available, without applying padding on the left and right side.

    1. About javascript, it is considered a best practice to edit style by adding and removing classes from elements. As I can read, you switch colors by adding style directly on the html file which can make things more complicated. What you can do instead is to add a .ratingBtnSelected class with all the rules that an active button should have. Then all you have to do with javascript is to add the class to the selected button and remove the class from unselected buttons. You can see my code as an example in my github repo (https://github.com/chrizgx/interactive-rating-component-main/blob/master/main.js) inside the 'rating' function.

    2. Personally, I don't know whether using <select> or <option> is better in terms of best practices. I have read that buttons are mainly used for submitting forms, but I think I am missing the whole context.

    I hope you'll find my feedback useful. Happy coding and have fun exploring. Feel free to ask anything, I'll be happy to help!

    Marked as helpful
    0
  • Chris•180
    @chrizgxPosted almost 2 years ago

    Hello Ariya, and congrats on submitting your first project. I took a look at the design and the code, and they both seem pretty well structured. I am writing this feedback to provide you some tips for further development.

    1. You can use @media query on CSS to change styling based on device width (https://developer.mozilla.org/en-US/docs/Web/CSS/@media/width). The only thing you'll have to adapt is 'flex-direction' property (you have to set it to 'column', in order for the image to be on top of the .content-div element). Don't forget to alter border-radius values on '.img-div img' and '.content-div'

    2. You can use 'transition' property on '.cart-btn' to include some kind of animation when the user hovers on the button. All it does is smoothly transition from one color to another.

    3. Lastly, you can use variable names for colors to make your life easier. I really like that feature because I can give a name to every color (such as '--main-color', '--secondary-color', '--text-color', etc.) and stop copy&pasting color values all the time. It is also useful in case you want to change the page's main colors, because all you have to do is change the variable value. You can find more about how to use CSS variables here: https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties

    I hope you'll find my feedback useful. Have fun coding and exploring new concepts.

    Marked as helpful
    0