Looking to hire developers?
@rostyslav-nazarenko
@Udy-Johnson10
Submitted
I had difficult with aligning the prices to the center and the right side height wasn't aligning with the left. I'm not sure of the positioning code that why I didn't use it. Also not sure whether the site adapts to any display screen properly.
@rostyslav-nazarenko
Posted
Hello! Nicely done!
main and footer for semantics. Change <div class="section"> to <main class="section">, and <div class="attribution"> to <footer class="attribution">.picture element for providing different images to the different viewport widths, plus it has a lot more uses, check it up.id for styling, use only classes.Your CSS has some issues, keep learning and you will notice them yourself. Feel free to look at other people's solutions for learning purposes.
Keep coding! Have a great day!
Marked as helpful
@Grendaizo90
Submitted
Still have a problem with responsive behavior, maybe you could tell me about bad practices in my code, and about best pratices i should use.
Thanks in advance!!!
@rostyslav-nazarenko
Posted
Hi!
Everything looks great! Only a few suggestions.
button as it doesn't inherit font properties from parent elements.alt attribute for the image or leave it emptyem units. The same goes for component max-widthstrong for styling, use CSS.@ElhabibTOUAOUA
Submitted
@rostyslav-nazarenko
Posted
Hi! Great solution
There're issues to resolve and a few suggestions
alt attribute should describe the image or its function, and be empty if is only decorative. alt="Card Hero Image" gives no information. Don't use word image in alt, screen readers will read img as Image, Card Hero Image. The same with the music icon.html {font-size: 62.5%;}. Read this article for more detailed information.max-width: 144rem; margin: 0 auto; to .container makes no sense as it does nothing.font from parent elements because font-family is right now set to default ArialKeep coding!👍
@mtaman
Submitted
What did you find difficult while building the project? -- I learn that <picture> git line height from the parent - I had white space after <picture> and I fix it with add line-height: 0; to the parent tag.
Do you have any questions about best practices? it will be nice to know Your feedback.
Thank you
@rostyslav-nazarenko
Posted
Hi! Great result! 😀
Just a few suggestions/issues. I'm only studying so take my advice with a grant of salt.
img is an inline element, most people use resets to make it display as block element that way there's no need in setting line-height to 0, especially when you need then to set it again on text elementsmin-height: 100vh; to the parent element. You don't need to repeat it on body element and on main element, use only on main. And remove min-width from both of them.prouduct_outer .prouduct_body .old_price {} is the same as .old_price{}.@YagyeshMani1498
Submitted
Feedback welcome 🙂🙂
@rostyslav-nazarenko
Posted
Hi! Nice and clean code!
<div class="container"> to <main class="container">`picture element too). Change the font size in the browser to 18px and you will see the component break.height to min-height on the body element.overflow-x: hidden; to overflow: hidden;Marked as helpful
@Zeke-Cachi
Submitted
NFT preview card, made with HTML and CSS only. Had to use a bit of sass to make the eye and cyan color appear at the same time on hover, since I couldn´t figure out how to place the selectors with vanilla CSS to make it happen
@rostyslav-nazarenko
Posted
Hi! Fantasticsolution
main element, and use h1 instead of h2min-height on bodyemMarked as helpful
@jrleijnse
Submitted
@rostyslav-nazarenko
Posted
Hi! Awesome solution! 😁
html element`. Part of the card is cut. Try turning off and on style in dev tools.@TanutsaponJ
Submitted
@rostyslav-nazarenko
Posted
Hi! Great code!
main element only card and leave attribution on its own in the footer tag.visually-hidden text is the sameHave a wonderful day!
Marked as helpful
@VasJM
Submitted
This is my first Frontend Mentor project and it was heaps of fun since it's been a while (read: 4 years) since I've built anything! Who knew trying to figure out spacing and font-sizes from just a jpg was going to be this hard? 😂 I'm sure I'll get better at it!
I didn't come across any issues, but if someone could comment on how I structured and annotated my code, that would be great! I just want to know if I'm on the right path here. Although if there are any other issues, feel free to point those out, too! :)
ETA: Okay, so I just saw the FM image preview and it looks all wack, which is weird considering it looks perfect on my browser :(
ETA: I figured out the issue! Well, not an issue really. It's just that the default font size on my browser is set to 12px (my personal preference) instead of 16px, hence the supersized look. Should I change this?
ETA: I fixed it!!!
@rostyslav-nazarenko
Posted
Hi, Vas! Your code is spectacular! 😍
There is one issue that I found and trust that was hard because everything is perfect.
Change px to rem or em for the max-width of the card, for media queries, and for a media query in the picture element. For media queries is better to use em. Why do I consider it an issue? If you slightly increase the font size in the settings card breaks.
Marked as helpful
@lucassantosdl
Submitted
@rostyslav-nazarenko
Posted
Hi!
A little bit of the same as in the previous challenge.
main and h1img element should always have an alt attribute, if the image is purely decorative leave it empty alt=""font-weight should be without units (line 91 of style.css)Keep coding!
Marked as helpful
@lucassantosdl
Submitted
@rostyslav-nazarenko
Posted
Hi! Cool code!
I have a few suggestions.
main element for accessibility. Change <div class="container">...</div> to <main class="container">...</main>alt attribute should describe an image or its function. Don't use the word image because screen readers read the tag as "image". In this case use QR code to frontendmentor.io. Read this article for more informationh1 instead of h2, don't skip levelsmax-width: 100% on imagesHope I've been helpful!
@ClaudioAmareno
Submitted
Hi! I'll be glad for your review and tips how to improve my code in html and sass :)
@rostyslav-nazarenko
Posted
Hi! Amazing solution! 😍
I will try to provide feedback, but it is mostly tiny bits here and there.
height: 100vh to min-height: 100vh. If users set a slightly bigger font size they wouldn't be able to scroll and see the whole component.h1 for this but here it is a stand-alone project. Remove p element from button.em for media queries too.Have a nice day!
Marked as helpful
@liammalby
Submitted
@rostyslav-nazarenko
Posted
Hi! Great code!
I found only a few issues with using absolute and relative units. I will explain as much as I can but here is a great article if you need more information.
em or rem. Using rem is easier as it is based on the root font size which is by default 16px, but users can change it in the browser settings. em is based on the parent element font size that way it is harder to set precise font size, read more in the article.body { font-size: 15pt; }. pt is an absolute unit, which is used in print. In this case set font-size to 0,9375rem (15 / 16, 15 is your target size, and 16 is the default).p element to 1rem (and you override previous rule) which is 16px (by default) and h1 to 22px. This makes p change its font size according to user preferences and h1 would be stuck and not change its font size.line-height: calc(1.27 * var(--header-font-size)); and line-heigth: 1.27; mean exactly the same.So, generally, people set font in rem to the body element which makes other elements inherit from body
Marked as helpful
I've changed my very first project according to some good advice. This time, I can't understand why my text area is higher than the design file's text area. I checked my elements positions and they are in the same positions as the figma files. What should I do? Thanks.
Also, are the nested structure of the semantic containing elements and BEM notation correct?
Thanks again.
@rostyslav-nazarenko
Posted
Hi again!
Mostly the same issues read my comment on your previous challenge.
I noticed only one thing. Don't set width and height to card element. Right now it is completely fine because you have a fixed font size and a card with a fixed size. But later on, when you will work on bigger challenges or projects it might create problems, like overflows, text on top of another text. As a quick fix, use rem units for width and don't use height at all
Marked as helpful
@rostyslav-nazarenko
Posted
Hi! Great solution!
A few suggestions.
article and section. I, myself, still don't understand how to use them correctly. Here is a great article. That is why HTML validation report have 2 unresolved issueshtml { font-size: 16px; } because it destroys your later use of relative units. By default in most browsers font size is 16px and setting it to absolute units prevents people from setting their own font size.Have a great day!
Marked as helpful
@asekabtw
Submitted
Hey guys! I am uploading new challenge! I hope I will get some feedback from you. And also how can I style fonts on this website? I mean frontendmentor.io Cause somebody is using bold, bullet points and etc.
Have a great day!
@rostyslav-nazarenko
Posted
Hi! Great solution!
I have almost nothing to add, just look at the picture element for providing different images at different breakpoints. Hopefully, others will provide more useful feedback 😁
To answer your question. It is called Markdown (.md) it has many uses and different structures which depend on the website or app that uses it. At frontendmentor.io you can see at the top of the feedback box MARKDOWN HELP. It contains all the syntax. It is recommended that all GitHub repositories should have README.md file. Markdown there is more extensive than here.
Have a great day!
Marked as helpful
@MarcosMeloPyRu
Submitted
Reuploading because the reports weren't updating...
@rostyslav-nazarenko
Posted
Hello!
Great solution! A few suggestions.
main element or rename div class="box" to main class="box". Use h1 instead of general div element<br> because screen readers will make pause in these places which will break the whole message, just let the text wrap by itself or use padding/margin to limit available spacewidth and height attributes in img element, specify those with CSS like this max-width: 100% that way image will occupy available space and resize according to it* { margin: auto; }. It can create problems later on because in different cases margin: auto; behaves differently. Plus you use margin afterward and thus you overide previous rule.background-color for each element as by default it is set to transparentwidth and height of card component, set only width or better max-width.boxGreat job! Goodbye!
Marked as helpful
@alexsanderBDS
Submitted
@rostyslav-nazarenko
Posted
Hi again! Great solution!
Please check my feedback on your previous challenge, rather than that I can only recommend you read article about Alternative Text
Keep it up! 👍
@alexsanderBDS
Submitted
@rostyslav-nazarenko
Posted
Hi, Alexsander!
main tags or change div class="container" to main="container::before pseudo-selector 👍h1 instead of h2del or s for the old price, plus some read-only text to provide more informationfont-size: 62.5%; read about it heremin-height: 100vh on the body element to allow scrolling, if you try to resize the window you will see thathead element so there wouldn't be additional calls to the serverKeep it up! Can wait for your next solutions!
Marked as helpful
@Manuelcoder02
Submitted
How can I improve on the media query section of css?
@rostyslav-nazarenko
Posted
Hi!
I would recommend starting from more simple projects like QR card If you feel like you still don't know enough [freeCodeCamp]{https://www.freecodecamp.org/learn/} is a great free resource for learning HTML and CSS
Practice makes perfect. Do not be discouraged!
Marked as helpful
@Ago-oyu
Submitted
@rostyslav-nazarenko
Posted
Hi! Great code!
Just a few things to point out
picture element can only contain one img element and as many as you need source elements. Change it to general divdiv to footerbody main .preview-card .nft__info .flexbox-item .nft__time-left p {}
.nft__time-left p {}
body and main selectors alone and then renaming .preview-card to .nft so you can target elements by specifying the next part with &
Something like that
.nft {
background-color: white;
&__image {}
&__info {}
}
.nft, nft__image, and nft__infoI'm still learn SCSS but I hope my feedback is helpful
Have a excellent day!
@kloprop
Submitted
Hello, everyone. This is my second sharing in frontend mentor. It would be helpful if I got any comments to improve the code. Also, I faced some issues when I was coding the nft card. It would be great if it got answered. THX
1.Should I wrap every image into a container for styling the image instead of adding to the image directly, like the size or the border radius?
2.Are there any difference between adding the svg under <svg> tag and <image> tag ?
@rostyslav-nazarenko
Posted
Hi!
main and footer.alt attribute is okay for icons but add some description to the main image and avatar image.font-size: 62.5%; I wrote in your previous challenge.To your questions
img element, but they nest them in divs for additional styling or for adding Javascript.svg allows you to change the color of SVG with CSS or to use Javascript to add change SVG in real-time (from style to shape)I hope I helped!
Bey!
Marked as helpful
@kloprop
Submitted
@rostyslav-nazarenko
Posted
Hi! Excellent solution! 🙂
main tag or change div class="card" to main class="card"div class="attribution" to footer class="attribution"alt should describe an image or its function and be empty only if it is only decorative, read more about alternative text here. In this challenge alt can be QR code to frontendmentor.io so you can show the image function in this casefont-size: 62.5%; read about it heremin-height: 100vh; on body element so that in case of overflow it will allow scrollingHave a nice day👋!
Marked as helpful
@manjeshrv592
Submitted
@rostyslav-nazarenko
Posted
Hello! Excellent solution!
span elements to div elements. You can remove nested divs, svg and text are inline elements so they would be next to each other by default, and no other styling to neededhtml: {font-size: 62.5%;} you can read about it hereHave a fantastic day!
Marked as helpful