@jblaszak
Submitted
Thanks for the help @SHMITEnZ and @hazel79!
Looking to hire developers?
@OmarMAttia7
@jblaszak
Submitted
Thanks for the help @SHMITEnZ and @hazel79!
@OmarMAttia7
Posted
Hello, congratulations on finishing the challenge and submitting your solution.
<picture>
element is a good solution for responsive images in HTML, to learn more about it in depth I suggest you read this guide by MDN.<main>
element since it is the only thing on the page, however if you have a bigger document with more content you would probably wrap it in a <section>
or <article>
or it would be in a bigger component wrapped in one of these..card
class has an overflow: hidden
attribute, this will prevent people from scrolling on shorter screens, try decreasing your browser window's vertical size to see for yourself. you should remove it, I'm curious as to why you added it?Otherwise you did a fantastic job, good luck and keep coding :).
Marked as helpful
@Abrosss
Submitted
Hello. I am not sure if height and width of the container should be dynamic, depending on the random advice text or fixed?
@OmarMAttia7
Posted
Hey, good job you did there 👍.
Like @byronbyron said your question is not technical it's more concerned with design.
But in case you want to make it a fixed width you should go with a percentage for smaller screens and a reasonable max width for larger ones. If your component is a flex item use vw instead of percentages.
Marked as helpful
@kamrulsaad
Submitted
Can anyone suggest me how I can reduce the classes for buttons in the code?? for setting colour to the texts to be like that of the card I had to do in individually. Is there any easier way to do that?
@OmarMAttia7
Posted
Good job finishing the challenge 🎉✨.
Unfortunately there isn't a way to assign the color values automatically unless you're working from JavaScript or using a CSS preprocessor, you can however, make your code more manageable by naming the classes more clearly, for example like this .btn-orange
, .btn-cyan
and .btn-darkcyan
instead of just .btn-(1,2,3...)
.
Marked as helpful
I would really love to figure out how to make this responsive without using media queries, because as it is the card works at 1440px and 375px with some super janky results in between and lower than 375.
The margin between each section from Order Summary down changes at different rates, and the font sizes change so subtly I'm not sure how to create the exact dimensions of the mobile version using things like min(), max(), or clamp.
@OmarMAttia7
Posted
Congratulations on finishing the challenge 🎉. Good job!
I'm not entirely sure I got your question right, what exactly do you dislike about the current design? I'm not sure I see a problem here but I'm new to responsive design as well. You shouldn't be concerned with devices smaller than 320px in width, in the rare case someone uses a device like that they could just scroll horizontally.
I also noticed that you're using exact measurements with pixels, in the case you want to change measurements on different screen widths you should use rem
s as reference for your measurements and then just changing the font-size of the html
element at different media queries and adjusting the font-size
of text elements if they get too small or big you could look here for more information on how rem works.
Also you should turn height: 100vh
to min-height: 100vh
in your container because it breaks your design on landscape orientation and other small heights.
Try looking into working with a mobile-first approach to responsive design it could make things clearer for you.
Marked as helpful
@elyssontanaka
Submitted
Hi guys!! This is my first challenge, I would appreciate any comments or suggestions to help improve the quality of my code or on any mistakes you find, especially on the responsiveness part.
Thanks!
@OmarMAttia7
Posted
Congratulations on finishing the challenge 🎉. Good job! I have some feedback on your solution.
First I would like to point out that your usage of the <section>
element is a little confusing, you should use a <div>
for styling purposes. Check this page for information on how to use it correctly.
Second thing I noticed is that you're way too focused on getting the design pixel perfect, you should be more focused on things like layout and accessibility, how would your component work when put in the same space with other similar components?, will it work correctly if put in another document with other content?, will it resize correctly if a visually impaired user increased the font size?, If someone is using a screen reader will they be able to navigate it correctly?.
By focusing too much on getting the design perfectly you are missing on other important things.
I suggest using flex to center things on the page instead of position: absolute
and using em
s instead of pixels for starters.
As for responsiveness, you worked on the desktop and large screens first and then made appropriate media queries for mobile and other smaller screens, that works fine most of the time. but you usually want to work with a mobile-first workflow where you make sure your code is working correctly on smaller screens and then expanding from there as it gives you more freedom and takes much less effort, check this article for more information on the subject.
Marked as helpful
@karamthedev
Submitted
I would really appreciate your feedback!
@OmarMAttia7
Posted
Congratulations on finishing the challenge, good job that was solid :).
I have a couple of things to point out:
<main>
element or something equivalent.Good luck and keep coding <3.
Marked as helpful
@leorichy
Submitted
All reviews are kindly welcomed
Thanks 😍
@OmarMAttia7
Posted
Congratulations on finishing the challenge :). Your is code has some problems with HTML Markup and CSS:
<main>
element or something equivalent, in this case it's preferable that you give it class="container"
instead and delete the <div class="container">
you have as it has no usealt
attribute of your images should have a meaningful description of the image for people who can't view it visually, you shouldn't leave them empty.<h1>
element, in this case your Order Summary should be an <h1>
instead of an <h3>
, you can change the font-size with CSS.<button>
s or <a>
s to avoid accessibility issues.<img>
as it is there for decoration.@RonanCa
Submitted
Hey ! I'm open to any suggestion, I struggled a little bit with the hover effect on the main image.
@OmarMAttia7
Posted
Congratulations on finishing the challenge :). You have some problems with HTML markup, most of them are in the report:
alt
attribute that describes the image for people who can't view the image visually.<main>
element, it's preferable that you give it the class="container"
instead and remove the <div class="container">
as it has no use.<h1>
heading element, in your case you should put <a class='title'>Equilibrium #3429</a>
inside an <h1>
.Good luck and keep coding <3.
Marked as helpful
@ingnall
Submitted
@OmarMAttia7
Posted
Congratulations on finishing the challenge :). You have a few problems with standard html5 semantic markup, most of them are in the report of your solution.
<img>
elements should have an alt
attribute that describes the image for people who can't view the images like so <img src="images/image-avatar.png" alt="Jules Wyvern">
.<main>
element or something equivalent, like so<main> <div class="card-main">
<h1>
element, in this case it should be <h1 class="nft-name"> Equilibrium #3429</h1>
you can adjust the font-size using css.@ahouyang
Submitted
Any general tips on code styling/best practices are always appreciated. I also used flexbox basically any time I needed to move a component around, wasn't sure if that's always ideal or not. Thanks!
@OmarMAttia7
Posted
Congratulations on finishing the challenge, Good job :).
I'd like to point out a couple of problems with your HTML markup:
<main>
element. Every HTML document should have a <main>
element.<h1>
element, also every HTML document should have one.Marked as helpful