@elaineleung
Posted
Hi Jumagobe, I think you made a really good attempt here, so well done, first of all! To be honest, I'm not sure where to start for feedback, as there's just a number of things you probably would need to work on, not just the JavaScript. I'll just note down my observations here:
-
It's best to use responsive images with either
img
andsrcset
or thepicture
element instead ofbackground-image
. This is meant to be a page that showcases the key products for a company; the visual aspects are one of the most important parts of the page, and so you'd definitely want the images to be available to the screen reader, so that the screen reader can convey what the images are using theimg
attributes. Thebackground-image
is really meant for images that are used as a background with text in the foreground. Another point about the images is that they look distorted and out of their aspect ratio right not, so be sure to fix that! -
Right now, the page only looks optimal at the desktop view on a fairly normal sized screen; when it gets smaller, things start to look out of shape and alignment, and they just end up looking broken. You also don't seem to have a mobile view here yet, but I see in your JS script that there should be a open and close hamburger toggle for the menu, so I'm not too sure what's happening with the mobile view. All I can see is that, it's not there right now, sadly!
-
It's good that you are able to get the buttons to work in changing the images, and as you said, the Javascript really does need work. I normally would spend the time to help with rewriting this, but I don't know where to start here because I would rewrite every line you have. What I suggest you do is that, in the challenge hub for this challenge, click on "Solutions" at the top right corner, and have a look at how others did this challenge. I think you'd get a lot from comparing other people's solutions.
Lastly, I would suggest that you work on some more Junior level landing pages first, and I'm going to see if I can give you some more feedback on your other solutions because I think you do need some help first before working on a project like this one. You need to work on using media queries, using a mobile-first approach, and also working on how to write Javascript.
Anyway, I really do hope you check out the other solutions to this challenge and see how to fix some of the problems here. Best of luck, and keep going!
@Jumanjigobez
Posted
@elaineleung I appreciate for this feedback but according to design the small screen devices should be at @media (max-width: 375px)
.
That's what I did for mobile viewers. Kindly check my css code.
@Jumanjigobez
Posted
@elaineleung I appreciate for this feedback but according to design the small screen devices should be at @media (max-width: 375px)
.
That's what I did for mobile viewers. Kindly check my css code.
@elaineleung
Posted
@Jumanjigobez I did check your code and saw the max-width: 375
before I submitted my comment, but things were looking too misaligned before that breakpoint that I just couldn't shrink the window down further to see the menu. I'm sorry I couldn't check the hamburger menu then, but this was honestly just hard to look at.
Each FEM challenge provides the mobile and desktop views, but the images are meant to be reference for how the site should look at that particular width, not that that width would be the max width. This is why as the developer, there needs to be some discernment and proper judgment in how the site should look optimally at most browser widths and screen sizes. In this solution I would say everything between the 375px to 830px point is not optimal, which is a pretty wide range, and it only starts looking optimal when the nav links are no longer wrapping to the next line. I suggest taking a mobile first approach so that you can slowly make changes that work towards the desktop view, and that is really much easier than going from a desktop view to the mobile one.
@Jumanjigobez
Posted
@elaineleung Let me try and place at 600px
maybe what would you suggest for best breakpoints for most used smartphones :)
@elaineleung
Posted
@Jumanjigobez Yes, see how that works for you, and keep experimenting! 🙂