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.
I think the card gets slightly too small on lower screen sizes.
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.
Any tips to improve the code are welcome! :)
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');
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
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
Any tips and tricks are welcome.
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.
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.
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:
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)
Flexbox froggy - https://flexboxfroggy.com/
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!
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.
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:
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.margin: 0 and padding: 0 in the :root?<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!
What type of methodologies is the most appropriate for give class names?
Most commonly used methodology is BEM - you can read about it here https://getbem.com/naming/
Any comments are welcome, project is finished using CSS HTML Media Querrys.
I think the code can be improved, right? Comment your suggestions, please.
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?
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?
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)
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.
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
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?
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.
