@tomxydSubmitted about 3 years ago
P
cjdemille
@cjdemilleAll comments
- @cjdemillePosted about 1 month ago
There are a couple steps you could take to refactor this.
Your span with the "header-1" class is unnecessary. You could just apply that class to the p tag.
Since the <b> tag doesn't have semantic meaning, you could just use the font-weight attribute in CSS.
You also may want to check your breakpoints for switching from the column, to the 2D grid. Your project jumps a bit early and the image is coming down further than the card.
Because you're manually setting the width and height (and changing the width in your media query), the images can distort as you scale the page.
0 - @paduiiiSubmitted about 1 month agoWhat challenges did you encounter, and how did you overcome them?
At first I can't move up the text "$169.99" so I have to try searching for solutions online usually I don't use position: relative; much but in this case it help me
@cjdemillePosted about 1 month agoThis is a good take on this project. You've obviously been able to closely replicate the design.
It would be best practice to break your styles out into a separate CSS file. That will help to make markup easier to read.
You can also use CSS to adjust the letter spacing in the word "perfume." This will allow screenreaders to still see "Perfume" as a single word.
It might be helpful to give your class names something with more meaning than just "card-1" and "card-2." It can be easier for someone else to use them, or for you to make adjustments to a files when you come back to it later.
You could also use flexbox to center the "$169.99" to the "$149.99". That would allow you to make sure they're always aligned and not dependent upon a user having the window at the precise size to get the desired alignment.
Marked as helpful0 - @DarkR4gerSubmitted 11 months ago
- my card width is not shrinking corresponding to the browser width, is there any problem in width
@cjdemillePosted 11 months agoYour card isn't shrinking based on the browser with because only max-width is set. When you use max-width, you're telling the browser that the element that it should use all of the available space until the element hits the max-width.
If you would like the element width to be proportional to the browser width (until it hits the max-width) you can use the width property along with max-width.
For example, you could set the width to 80% and then the element would take up 80% of the available space until it hits your max-width value and then it will stop growing.
If you choose to go that route, you would need to update the width property on your course-img to be dynamic (i.e. a percentage) because that currently has a fixed width.
0 - @megaieraSubmitted 11 months ago
this is my project, i'm sure my css could be better. i tried using grid at some point but it was nightmare-ish to say the least. i don't know what i was doing wrong, but i'll try again with a different project. i can't place the attributions where i wanted to, if anyone could tell me why justify-content and align-items don't work at all that'd be cool.
is there anything i should do differently in the future? good habits to get? thanks in advance.
@cjdemillePosted 11 months agoWhen you wrote out your reset css, you set the margin for all items to 0 (which is helpful for overwriting default styles) but in this case, that means there's no margin between your card and the attribution div.
You could choose to add a margin-top to your attribution div or a margin-bottom to your main-container div.
0 - @meisteraxelSubmitted 11 months ago
Hello! :)
Any tips are appreciated! What should I do better next time? Struggled a little bit with the height Attribute of the body element because Content would cut off after changing from Rows to Columns in the Grid Container.
I do not have Pro, so the Design isn't 100% correct because i do not have access to the Figma Files.
Thanks in advance! Axel :)
@cjdemillePosted 11 months agoI actually don't think you need to set the height for your main container. I tested in dev tools without the height set and it works just fine without it (it also saves the headache of the content overflowing and needing to fiddle with a set height).
This looks great for mobile and desktop, but it does get cut off at screen sizes in between. Your first breakpoint is at 425px, but once it changes to the desktop layout it cuts off the outside portions of the outer boxes until the screen size is around 670px. You may want to consider changing that breakpoint, or working through a solution for that range.
Marked as helpful0 - @ArnaudM1983Submitted 11 months ago@cjdemillePosted 11 months ago
This is very well done. If you wanted to center the card on the page, you could make the body element a flexbox and use the justify-content and align-items properties to center.
You could also make the card's position value absolute, set the top and left properties to 50%, and then set the transform property to translate(-50%, -50%) to center it.
Marked as helpful0 - @angry-wolf-sketchSubmitted over 2 years ago
Challenge was quite easier than i expected. However i got stuck at Responsive Resize using just CSS.
@cjdemillePosted over 2 years agoThis is a really good first project. It displays and scales well.
It doesn't have a huge impact on this project, but it would be cleaner code to wrap the card in a "main" tag.
You might want to look into centering the card vertically (like in the example). I usually find it easiest to use flexbox for this.
Marked as helpful0 - @yonas-fsahaSubmitted over 2 years ago
Guys this is my first project in frontend mentor; please give me feedback and comment
@cjdemillePosted over 2 years agoThis is pretty good for your first project! The biggest thing off the bat is the aspect ratio and spacing. The card looks like it could use more white space on the whole and the box with the plan information can definitely use some padding to make it more readable.
On mobile, the white card containing the content gets smaller and the "proceed to payment" takes up the entire width of the screen. Adding some padding to the card might help with that.
Also, the "proceed to payment" button might be more user-friendly if it was a bit taller. I usually try to shoot for buttons on mobile to be about thumb-sized.
On the card, you could use an h2 header instead of an h4 and use CSS to style it apporpriately. It will stick to the standard heading hierarchy by only incrementing by one.
One thing on code readability, it looks like you used "dev" instead of "div" in your HTML. It's obviously working and displaying properly, but it did make me do a double take.
Marked as helpful0 - @MosopeoluwaSubmitted over 2 years ago@cjdemillePosted over 2 years ago
The link to your sedan icon is broken (which looks like it's also causing an alignment issue in your webpage).
I'd also explore using a flex-box or bootstrap framework to display this. It would make it easier to center the row of cards horizontally. It would also help with the responsive layout.
Because you set those cards to inline-block you have an awkward layout between the mobile and desktop views. On a tablet or medium size window the sedan and suv sections each take up half a row, but the luxury card has the same dimensions but is centered on its own row. You could use bootstrap or flexbox with a media query to get a better responsive layout.
0 - @manali-cseSubmitted over 2 years ago
How can I make it better?
@cjdemillePosted over 2 years agoThis is really close to the desktop version of the challenge.
The biggest thing here from a design point is that the difference in the color on the callout text ("insights") and the overlay on the image is throwing me off. You can create an overlay of just that purple color.
Create a new div (I'd call it container) and wrap the header image in that. In that same div, create another div with no content (I'd call it overlay).
In your CSS, set the image's position to relative. set the height and width of the overlay div to 100%, give it a background-color of the purple (make it an hsla instead of hsl and play with the alpha channel) set the display to absolute, top to 0 and left to 0, and the z-index to something greater than 1. (Sometimes the overlay will be slightly bigger than the image itself. to correct this, set the "font-size" on the container div to 0.
This site isn't responsive, which is what the challenge calls for. You could solve that pretty easily by adding in some media queries or a framework like bootstraps. I'd either lean into bootstraps or flex-boxes so that you can easily switch the order of the content when change screen sizes (I'd probably set the flex-direction to column on mobile and then row-reverse on desktop)
Marked as helpful1 - @dodrinSubmitted over 2 years ago
Hi, Im newbie coder who want to become front end dev. In this project I have a few difficulties that I couldn't figure out.
- Overlay is somehow not matching with the image position.
- I want the card to be vertically centered when opened with smaller display.
I appreciate your help and feedback on my project. Thanks!
@cjdemillePosted over 2 years agoThe overlay can be way trickier than you'd expect for being pretty simple CSS to write. One trick that will work with your code would be to set the font-size property on your .main-image class to 0.
It seems weird because there's no text in the image, but I remembered that when I was taking classes, that a return in your HTML can become a space.
It looked like your .main-image element was a couple pixels larger than the image, so I tried it out in dev tools and it worked.
It looks like you've already corrected the issue with verticle centering. I almost always forget to set the cross axis height of a flex element, but setting a height and width and then using the display of flex and justify-content: center and align-items: center should do the trick every time.
1 - @Crab-ProgSubmitted over 2 years ago
I already know there are some problems but I couldn't find a way to correct it. When you hover the picture, i put it in green since I didn't know how to change it into a lightblue and I couldn't add the icon on the picture. Also my blue line is going from all the width... If you have any advices, feel free to help me! Thanks.
@cjdemillePosted over 2 years agoHover problem—you can create a div with the eye icon that's has a background color of light blue. I like to wrap the image and the overlay color in a div so it's easier to size the overlay on the image and set the overlay to absolute. Then you can use a pseudoclass so that the opacity is 0 until you hover.
For the line at the bottom, you can wrap the cost and time components in a div and set the border-bottom property.
The other thing to watch out for is that you didn't include alt-text on the images. Ideally, you should be able to use a couple of words of descriptive text. If the image is purely decorative, you can set the alt-text to null by setting it to "" with no space between the quotation marks.
0
