Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • Adrian• 270

    @EidrianMax

    Submitted

    Simple card that i made with flexbox and also i use media queries for that the designs become adaptable.

    I use flexbox because i use gap instead of margin to separate elements.

    Some feedback and advice are welcome.

    NFT Card made with flexbox layout and media queries

    1

    Fancy• 320

    @maciejkrol18

    Posted

    1. I think the card gets slightly too small on lower screen sizes.

    2. You've done a good job with applying aria-hidden to images that do not need to be read by assistive technology out loud. However, your project still lacks basic accessibility. Remember to use semantic HTML and change the div with the class of card to a main tag - Frontend Mentor's accessibility report points this issue out.

    0
  • Fancy• 320

    @maciejkrol18

    Posted

    I see that in your CSS code you include a CSS reset at the beginning. It's better practice to link such reset instead of putting it directly in your code. If i'm not mistaken, the css reset you're using is this one https://github.com/hankchizljaw/modern-css-reset.

    You can either link it in the <head> of your HTML document with the <link> tag, like so: <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/modern-css-reset/dist/reset.min.css" />.

    Or by using the css @import rule at the top of your css document, like so: @import url('https://cdn.jsdelivr.net/npm/modern-css-reset/dist/reset.min.css');

    Marked as helpful

    0
  • threshold• 50

    @thresholdtech

    Submitted

    Hi Guys,

    this is my fifth project. Well, it's so confusing tbh, and here's my result. I use 'grid' here, to divide the image and the text, I don't know if that is the right choice. I have an issue here, the discount price won't be centered when I use vertical-align: middle, any adv? TIA

    Product Preview Card Component (Perfume)

    3

    Fancy• 320

    @maciejkrol18

    Posted

    Centering the discount price

    The easiest way to solve your problem would be to make the div with the class of price-remark a flex container, and then apply align-items: center; to it, so that everything is centered vertically. Then, you can either apply a small left margin to the discounted price or use gap property on the flex container itself. You shouldn't use the vertical-align property in this case.

    Frontend Mentor's HTML validation report

    You've used backslashes to link the images (\). You should use normal slashes (/) instead.

    Frontend Mentor's Accessibility report

    All content should be contained by landmarks

    Look up Semantic HTML - https://www.w3schools.com/html/html5_semantic_elements.asp You should use a <main> tag instead of a simple <div> to improve the site's accessibility.

    Headings should only increase by one

    The perfume's name is a h1 and then you jump to h4 with the perfume's description. On that note, you shouldn't use a heading tag for a description. I would change it to a paragraph tag.

    Other

    Fix your "Add to cart" button's background color and add the hover state

    0
  • Fancy• 320

    @maciejkrol18

    Posted

    I would suggest improving the hr tag's appearance. As of now, it's very bold and doesn't really match the provided design.

    You can style the hr tag with the border property. The line will look way better with something like border: 1px solid #e3e3e3; applied. You can also try to use other color models such as hsl instead of hex code.

    Frontend Mentor's accessibility report mentions that all content should be contained by landmarks. You can fix that by changing the div with the class of container that contains the card to a main tag. Look into Semantic HTML.

    0
  • Alan• 10

    @Vaphu

    Submitted

    Hello,

    As I just recently started learning to code (like a week ago), and this challenge being my first project completed all by myself I would highly appreciate any feedback about my code. Is it well written? Is there anything I should fix/change?

    I haven't touched mobile part of the challenge because I wasn't sure how to approach it yet.

    3-column preview card component

    1

    Fancy• 320

    @maciejkrol18

    Posted

    While your solution looks good and close to the design, your code has lots of bad practices.

    Learn flexbox. Really. Right now you're positioning everything with tons of margins and almost everything has a position of absolute and a fixed width, which is like the worst thing you can do. Flexbox is a pretty easy tool for creating layouts - you'll find tons of free resources to learn it online.

    Some resources i personally recommend:

    1. Free flexbox course on Scrimba - https://scrimba.com/learn/flexbox (You'll find tons of other amazing frontend courses on Scrimba too - i would highly recommend checking them out)

    2. Flexbox froggy - https://flexboxfroggy.com/

    3. Flexbox guide on csstricks - https://css-tricks.com/snippets/css/a-guide-to-flexbox/

    As for touching the mobile part of the challange, hold onto it for now - learn the basics first. Good luck!

    Marked as helpful

    0
  • AasishSapkota• 60

    @AasishSapkota

    Submitted

    it was quiet a good challenge for first timer's like me it is very good . I don't know what to do with that attribute in the given code so i commented in the code you can see it and tell me . it's rough coding can you suggest me how can i improve my code and making it clean code for others to read it and understand . i am open minded so any suggestions you are thinking of giving fell free to comment.

    All feedback is welcome Thank you in advance.

    NFT-preview-card-component

    3

    Fancy• 320

    @maciejkrol18

    Posted

    The 'attribution', or 'attribute' as you referred to it, is something you'll find pre-written in the HTML file of every Frontend Mentor project. It's just a suggested way of signing your work, so to speak. You can delete / comment it out if you want.

    As for your code, it's pretty readable, although i've got some suggestions for you to consider:

    1. Why not simplify that article tag on line 37 in your .html file? It's not really readable when everything's on the same line.
    2. Remove the default margin from your body tag (add margin: 0) and add min-height: 100vh to it. The vh unit means viewport height, so your body tag will take up the entire screen, which makes the card properly centered.
    3. Some of your CSS variables are lowercase and other ones are camel case. You should make it consistent by picking one of the two. On the topic of variables, what is that margin: 0 and padding: 0 in the :root?
    4. I'm not really sure about that <ul> tag used in the nft's price and days left. I would replace it with simple flex container.

    That's all i could think of for now. Overall, good job on your first project!

    Marked as helpful

    0
  • Fancy• 320

    @maciejkrol18

    Posted

    Most commonly used methodology is BEM - you can read about it here https://getbem.com/naming/

    Marked as helpful

    1
  • Fancy• 320

    @maciejkrol18

    Posted

    Space out your CSS rules for better readability

    0
  • Fancy• 320

    @maciejkrol18

    Posted

    The code is overall pretty clean, point for using BEM in class names. However, please space out all of the stuff in your CSS file. It's very hard to read it as of now. I would also make all of the variable names lowercase - is there any reason color related variables are capitalized and all of the other ones are lowercase?

    0
  • Salvinas• 30

    @Salvinast

    Submitted

    Hey, the icon and the text on the button are not centered. I was wondering maybe I should somehow use flexbox to fix it. What do you think?

    product preview card component

    2

    Fancy• 320

    @maciejkrol18

    Posted

    Yes, you can easily fix it with flexbox. On the other hand, why is the cart icon outside of the button? Remove the transform: translate(46px, 36px); from the cart icon and get it inside of the button. Then apply following styles to the button

    display: flex;
align-items: center;
justify-content: center;

    This way everything will be centered, but the icon will be sticked to the 'Add to cart' text. You can fix it by either applying a small margin-right to the image or applying a small gap to the button itself (for example gap: 10px; - in case you haven't heard of gap, it's a flexbox property defining a gap between every item in a flex container)

    Marked as helpful

    0
  • Bekir Kutluhan• 10

    @BekirKutluhan

    Submitted

    I had hard times while trying to place background images :) I'm still unsure that if texts are correctly typed or is something missing? I would love to learn how to place background images responsive.

    ProfileCardComponent-BekirKutluhanCoskun

    1

    Fancy• 320

    @maciejkrol18

    Posted

    You placed the background images in a correct way 👍 In order to make the background images responsive, you need to create a media query in your .css file - a media query is a set of rules that apply while certain criteria is met, like the devices screen width. Media queries are a core element when it comes to making a site responsive. In order to apply different styling to different screen sizes, create a media query like so (by typing this stuff in at the end of your .css file, preferrably):

    @media screen and (max-width:810px) { 
      /* things go here, in your case... */
      .tlc {
           background-position:top /*value*/ left /*value*/, right /*value*/ bottom /*value*/;
      }
}

    Making it like so will apply the specified rules to the .tlc class when the screen width gets lower than 810px, which will be fine for mobile devices (change the value if you want). Tweak the background-position values so that the background looks like on the provided mobile design.

    One more suggestion, work on your file readability. Pay more attention to things like tabbing, spaces etc. Also some of your classes are empty with no rules set (for example the .child2 class) - do not use empty rulesets, you can remove those.

    There's lot of things to learn, definitely not bad for your first project here, good luck.

    Read more about media queries here if you want: https://www.w3schools.com/css/css_rwd_mediaqueries.asp You can learn tons of other things on w3schools, from the basics to advanced stuff as well

    0
  • impure• 40

    @z0dded

    Submitted

    The most troublesome thing for me was styling the definitely the .plans class because i didn't know exactly how to position the elements

    Did i do it right using position: absolute?

    order summary component with position absolute

    1

    Fancy• 320

    @maciejkrol18

    Posted

    position: absolute works in this example but personally i'd say it's slightly overcomplicated - there's no need to figure out the top, left etc. values when you can use something like flexbox instead. You'll be able to align everything more easily and precisely that way, in my opinion.

    Marked as helpful

    0