Benjamin Lincoln• 20
@CuriousFullStacks
Submitted
My only issue here is that I couldn't get the "M" in mentor to be a bit before the word "Scan" like in the preview. I'm not sure why.
P
Yasmine
@Yasmine10
All comments
- Charles• 170
@cacti00
Submitted
This was pretty easy to build
I have a question tho, apart from setting a flex-basis on the container to display the image and its contents side by side. Are there any other ways to set them side by side
I am open to any feedback on how I can improve and reduce unnecessary code.
Hi @cacti00
An easy way to display your image and content side by side is to use grid, you could do something like this in your
.product-cardclass:display: grid; grid-template-columns: repeat(2, 1fr); /* use grid-template-columns: 1fr; for mobile view */I also saw that you're using divs with an img inside to display the image in your html. There is an html element for that actually: '<picture>'
Here's an example of what that would look like for this project:
<picture> <source srcset="images/image-product-desktop.jpg" media="(min-width: 50em)" /> <img src="images/image-product-mobile.jpg" alt="" /> </picture>Hope this is helpful 😊
Marked as helpful
3 - bilguun1130• 380
@bilguun1130
Submitted
How do I decrease the size between the bottom of the card and creator's avatar and info? The very bottom part has too much space between elements but I don't know how to decrease that size. Tried margin-bottom and did not work. Anyway, this is the best I can do for this one.
Hi @bilguun1130
The reason for the spacing issue is because you used grid and then you downsized the icon image with percentages. It doesn't recognise that the icon image is not at 100% anymore, but if you use for example
height: 3remon the avatar image it should work just fine.To prevent that issue in the future you could also add
align-items: flex-startto your.main-contentclass, that way you can also see that the image is stillheight: 100%instead of 30%Otherwise great solution!
Hope this helps 😊
Marked as helpful
1 - atmaram-kambli• 60
@atmaram-kambli
Submitted
setting the main container box to the center of the height(screen)
Hi @atmaram-kambli,
Great job on your first project!
To center the main container you could change the following in your code:
- in the body:
body { font-family: 'Roboto Slab', serif; text-align: center; background-color: hsl(212, 45%, 89%); height: 100%; // -> remove this and replace with min-height: 100vh; display: flex; flex-direction: column; // -> remove this because this doesn't do anything align-items: center; justify-content: center; }- in your .main class:
.main { background-color: hsl(0, 0%, 100%); border: 2px solid red; border-radius: 8px; box-shadow: 0 4px 8px 0 rgba(0, 0, 0, 0.2), 0 6px 20px 0 rgba(0, 0, 0, 0.19); height: 450px; width: 350px; margin-top: 100px; -> remove this line display: flex; align-self: center; flex-direction: column; align-items: center; }I also saw that your qr code image is a little stretched, to fix that you could add the following in your css:
#qr { width: 100%; object-fit: cover; // -> add this line }Hope this helps!
Marked as helpful
0 - ClaireliseDev• 120
@ClaireLise-dev
Submitted
Here is my solution for the News homepage challenge!
I still need help for the mobile menu, I don't know how to make an hamburger menu with HTML/CSS so for now I just put the logo and hamburger icon menu!
Also I have difficulties to put everything in a 1440px screen without scrolling.
Any help is very welcome!
Hi @ClaireLise-dev 👋
Congratulations on finishing the challenge! It looks really good!
I don't see any problems with scrolling in a 1440px screen. To me it looks good.
For the mobile menu you actually don't need to write a second
<nav>like you have done. You can just use a media query and change the styling when it's below or above a certain breakpoint.And to make the mobile menu with only HTML and CSS, you can use a hidden checkbox with a label. In the label you place the hamburger icon and depending on when the checkbox is checked or not, you can show or hide the mobile menu. I've got a codepen that shows you how to do this 😊
Hope this helps!
Marked as helpful
0 - Demilade• 120
@Billygotclout
Submitted
Had issues making the nav bar responsive with pure css
Hi @Billygotclout 👋
Good job on finishing this project!
To make the navbar responsive with HTML and CSS you can use a hidden checkbox and a label. Inside the label you place the hamburger icon and depending on when the checkbox is checked or not you show or hide the navbar.
I also have a codepen that could help explain how to do this exactly 😊
Hope this helps!
0 @ondickson
Submitted
I have a problem on the mobile version. I could not get the responsive navbar to be like the image in the Figma without using JavaScript. I search and watched many tutorials but almost all of them was using JavaScript. Will be glad if anyone can help solve this bug with only *HTML & CSS *. Thank you
Hi @ondickson
Your solution is looking good!
I've got a codepen that shows you how you can achieve the mobile navigation with only HTML and CSS. The main thing is that you use a hidden checkbox with a label. The label is the hamburger icon and depending on when the checkbox is checked or not, you show or hide the navigation menu.
Hope this helps!
Marked as helpful
1- I'm using Andy Bell's Modern Reset with some minor changes. The reset contains the following bit:
/* Remove list styles on ul, ol elements with a list role, which suggests default styling will be removed */ ul[role='list'], ol[role='list'] { list-style: none; }Yet setting
roletolistis treated as a mistake in the report. Is it a bad idea to userole="list"to remove default styling?- There was no tablet view. It doesn't seem necessary but should it be added somehow anyway?
Hi Ania 👋🏻
-
When you're using
<ul>or<ol>a screenreader knows that that is a list because they're semantic elements that are used for lists. So like the report says,role="list"is unnecessary because it already knows it's a list without adding the role. It would be useful to add it when you're using a div as a list for example, because<div>is not a semantic html element and a screenreader won't know you're using it as a list. -
Also if you want to remove the list-style of
<ul>and<ol>, you could add the following instead of the role:ul[class]andol[class] -
And for the tablet view, it depends on how the website looks and where you placed a breakpoint. If you look at my solution you will see it's not really necessary in this case. If you just remove this line
grid-template-columns: 21rem;in.gridfor the mobile version, you don't really need a tablet view.
Hope this helps! Your solution looks great btw😊
Marked as helpful
0- Rajesh Janyani• 330
@Rajesh7rj
Submitted
Tried this challenge, face few difficulty, but I did it, but still face issue for setting border radius of product-mobile-image as per container border radius. I tried overflow : hidden as well but didn't work.
Every feedback is welcome....
Hi Rajesh 👋
You only have to set the border radius on <div class="row container">. When you then add the overflow:hidden it will cut off the corners of the image because those are overflowing the container. So if you remove border-radius: 3% 0 0 3%; from the <div class="col product"> it should still work.
Hope this helps!
0 - FalejevV• 500
@FalejevV
Submitted
(I'm just getting started. No idea what to even ask.)
I tried to make this card responsive and used a lot of @Media in css for that (A few every ~100px). Is this the correct approach? This amount of @media doesnt look very nice and correct.
Hi FalejevV 👋
Your solution looks great!
Regarding the media queries, I would recommend only adding one when it's necessary. In this case you only need one media query around 650px because the 2 columns are getting a bit small there.
Hope this helps!
1 Another attempt at intrinsic design. I think the layout is mostly working OK but have two issues that have flummoxed me that I would welcome advice on:
- The
h1should be white on the black background, and black otherwise. I have tried following some online tutorials usingmix-blend-modefor this but cannot make it work. - For some reason,
object-fitandobject-postioningdon't seem to have any impact on the map image. This is only really obvious on large screens, but again, I am a bit unsure of what I am doing wrong here.
EDIT:
- I just check how this looks on Safari - oh dear, some very storage behaviour going on in the header on the home page.
- And I realised I forgot to do the
srcsetfor mobile images. That is probably my least favourite part of HTML, so I think I'm going to leave it. It's not where I wanted to focus with this one! - And having just looked at the design and my attempt, I think I need to focus a bit more on combining this approach with 'pixel pretty close' if not perfect. I am way out here.
Hi Dave 👋
If you add width: 100% and min-height: 100% underneath the object-fit and object-positioning the map will fill the whole div.
Hope this helps!
Marked as helpful
1- The
- Pon Huang• 190
@ponhuang
Submitted
Change the default color to orange and blue, and add a light box-shadow design. But still couldn't do well in mobile version, and have problem to make the button work properly.
In the first time I use button element, then swap with a link, and it works slightly different. In what case we will use button, not a link?
Cause, the course I took before, they also make button by a link. And this confuses a little.
Hi Pon 👋
To fix the mobile version you just have to change the following:
.container { max-width: 63.5rem; // what you have now width: Min(90%, 63.5rem); // change it to this }The reason a button and a link work a little different is because their default styling is different.
Also here are some resources to better understand the difference between a link and a button.
- https://a11y-101.com/design/button-vs-link
- https://medium.com/design-code-repository/a-vs-button-b859547cae4d#:~:text=In%20%2C%20there%20is,%3D%E2%80%9Dbutton%E2%80%9D%20for%20semantic.
Hope this helps!
1 - Silasico• 40
@Silasico
Submitted
I would like to get your review and also corrections so I can become a better frontend developer and also to be able to master HTML, CSS and JavaScript and your review will really be of great help. Thanks in anticipation.
Hi Silasico 👋
Your solution looks great! I did see some small things you could improve.
- On the mobile version the image stretches when you size up the screen. To fix this you could add the following:
.product-img img { object-fit: cover; object-position: center; }- I saw the "add to cart" button keeps stretching when you go to bigger screen sizes, might be best to add a max-width to the button
- There are also some HTML and accessibility issues in the report that need fixing
Hope this helps!
Marked as helpful
1 - Brian Schooler• 440
@superschooler
Submitted
I had a hard time getting the image and text elements to both take up 50% width on the desktop view. I think if I use the background-image property in CSS instead of the image element in HTML it would allow me to more easily accomplish this without distorting the picture. Any other advice / suggestions?
Additionally, I had a hard time figuring out how to center the entire element vertically without setting a top margin with a viewport height attribute. Any thoughts on how to make this work?
Hi Brian 👋
- To center the component you just have to add the following in <body>:
align-items: center; height: 100vh;- One way of making the image and text each take up 50% would be the following code:
<article> <div class="image-container"> // ... (I would place the images inside <article> because it's a part of it </div> <div class="text"> // everything that is currently in <article> comes here </div> </article> //css article { display: grid; grid-template-columns: 1fr 1fr; } // or if you want to use flexbox: article { display: flex; } article .image-container { flex-basis: 50%; } article .text { flex-basis: 50%; }- Also a better way to switch to the desktop image would be to use a <picture> element in your html, you won't need a media query if you use that
<picture> <source srcset="images/image-product-desktop.jpg" media="(min-width: 650px)"> <img src="images/image-product-mobile.jpg" alt="" class="img-fluid" /> </picture>Hope this helps!
Marked as helpful
1 - Pon Huang• 190
@ponhuang
Submitted
Is there any better way to set the height of the content container? I'm thinking maybe shouldn't set the height and let it grow by its content.
Hi 👋 In my opinion there's nothing wrong with a fixed height on the content container.
I saw you have some accessibility issues that need fixing:
- Heading levels should only increase by one: for accessibility it's important that the headings are in a logical order so it's best to change your <h5> to <h2>
- All page content should be contained by landmarks: to fix this you need to surround your attribution in the html with <footer>
Also if you want the overlay of the image to be more similar to the design you could do the following:
background-color: #aa5cdb; background-image: url("images/image-header-desktop.jpg"); background-size: cover; background-blend-mode: multiply; opacity: 0.75;Hope this helps!
1 - Tanner Kopel• 320
@tankop1
Submitted
Does anyone have any advice for how to use the browser's local storage to save information from the app even after the page is reloaded? Also, there is a weird bug where after an invoice is edited, the edit, delete, and mark as paid buttons are moved upward. Any suggestions for how to stop this bug?
Also, is there any way to edit a JSON file with JavaScript? I feel like this would be an easier alternative instead of using a database to store data from this app.
Hi @tankop1, You could store the data in an array instead of a json file, that way you can edit your data.
Also the website looks great!
1
