Design comparison
Solution retrospective
Another challenge completed successfully.
Community feedback
- @AdrianoEscarabotePosted almost 2 years ago
Hi lucas cunha reis, how are you? I really liked the result of your project, but I have some tips that I think you will enjoy:
To prevent the background image from breaking at higher resolutions, we can prevent this in two different ways:
- Add a
background-repeat: repeat-x;
, the image will repeat on the horizontal axis, preventing it from breaking. - Add a
background-size: 100% 50vmin;
, the50vmin
will set its height as the page target, and100%
will make it stretch on the horizontal axis.
Feel free to choose one of the two!
To improve the responsiveness of the project, we can do the following:
.header-img-box img { width: 100%; }
The rest is great!
I hope it helps... π
1 - Add a
- @RichardOgujawaPosted almost 2 years ago
Hi Lucas!
Hope you're keeping well:) I really like your coding solution, especially the fact that you indented your code. I also like how you used an anchor tag for the button. I'd be tempted myself to use a button tag, but after doing some research into it it seems to be a pretty gray area, some people say you should never break away from semantic HTML and use an anchor tag in place of a button, and some would say that if the button results in a URL change, i.e. if it links to another document, then by all means wrap it in an anchor (for ex. here: https://www.ancestry.com/corporate/blog/buttons-vs-anchors) and style it like a button in CSS (for example in this article: https://accessibleweb.com/question-answer/is-it-ok-to-have-a-link-that-looks-like-a-button) I also appreciat the use of alt text, haven't really seen that being used in other solutions yet.
However, I do have some suggestions for your code. Then again these are just my opinion so feel free to take this on board or take it with a grain of salt, no hard feelings.
In the HTML:
- The main tag is for the main content on your site, but it's to be used as a container for it as opposed to being used directly on the content. The order summary card is the main content on the site so the <main> tag should be wrapped around it as a container. For the order summary element itself I'd recommend using an article tag, as article tags are used for elements that can be understood on their own without any information around them for context.
- I'd recommend looking into BEM Naming Conventions for naming classes. It makes it easier for you to write your class names without having to think too much about it, and it also makes it easier other developers to understand your classes, because the naming convention is quite a logical one. There are two articles I'd recommend reading on it, and once you read them you're pretty much set. The first one talks about how to write BEM (https://codeburst.io/understanding-css-bem-naming-convention-a8cca116d252 ) and the second one talks about common problems you may run into when writing BEM and how to overcome them (https://medium.com/fed-or-dead/battling-bem-5-common-problems-and-how-to-avoid-them-5bbd23dee319 )
- I'd recommend using the picture tag, it's better than just using the img tag. More on that here: https://blog.bitsrc.io/why-you-should-use-picture-tag-instead-of-img-tag-b9841e86bf8b
In the CSS:
- I'd recommend using :focus instead of :active alongside the :hover pseudo-selector, because the :hover and :focus do the same thing. This is when a user is either hovering over the element with a mouse or they have used the tab key on their keyboard to tab to it. In other words both of them refer to the moment(s) before the interaction with the element, the :active selector on the other hand is when the user has actually clicked on the element, and is therefore interacting with it.
- A faster way to write two pseudo-selectors like :hover and :focus together without doing .btn:hover, :btn:focus, is .btn:is(:focus, :hover). The :is pseudo-selector is a relatively new selector, which allows you to select multiple selectors. It does the same thing as another pseudo-selector called :where() the only difference is that :where() is easily overriden because it has a specificity of zero, and is takes the specificity of the selector in its list with the highest specificity. That's why where is typically left for situations when you want to easily override what you're about to write in the CSS rule, for example in a CSS reset. More on those two pseudo-selectors here: https://developer.mozilla.org/en-US/docs/Web/CSS/:where
- An easier way to center content in a container would be to use {display: grid; place-content: center;} on the container element.
Hope this helps:) And if you want me to explain anything a bit better just let me know, I was trying to keep this not too long, so I had to rush through some concepts.
0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord