This was an easy one to do, not much to say since it's a very simple and straight forward design.
I used flexbox to create 2 columns: one for the qr and the other for the text provided, nothing too fancy.
Any suggestions are as always, very much welcome!
Cheers!
Thanks to the three of you: Lucas, David and PhoenixDev22
I took your suggestions into account and applied them to the challenge. Most of them are things that I already knew but forgot to apply them 😅, like the main and footer landmarks or the margin for the container.
As Lucas pointed out, the div.desk element was clearly needless so I removed it and used flex's gap property to add some spacing between the elements.
Again thanks for the feedback!
Please feel free to critique.
Hey Roberto!
Firstly congrats on completing the challenge! Your solution looks good!
Here some pointers:
min-width: 1440px in the 1440px media-query. This is just streching the card way to muchdiv.main element, you can get rid of that height. Also when creating buttons, don't use like a height: 40px it's better if you use padding like a padding: 15pxThere is always room for more improvements but for me this are the more pressing ones in your case
Hope this helps,
Cheers!
PS.: You should also look at the report and try to fix the errors it tells you
Howdy!
(Feedback requests on the bottom)
Initially I wasn't sure what I could learn from this project as I have covered similar things in the past. Then it occured to me to build a custom search-modal and change the behaviour from the original design. I thought it'd be more interesting to build, sorry that it won't match the generated screenshot comparison 😁😅...
A few fun items to mention:
Ctrl/Cmd + S. I thought it only right to add this.Super short example you can run in codepen:
const filters= [1, 2, 3]
const data= [1,3,4,5,6]
const data2 = [1,6,8,4,2,3,9]
const checkMatch = (filterArray, dataArray) => {
return filterArray.every(filter => dataArray.includes(filter))
}
console.log(checkMatch(filters, data)) //returns false
console.log(checkMatch(filters, data2)) //returns true
I don't think that I spent a lot of time perfecting the design, I'll trust that it's close enough without too many errors.
If I need help with anything it'd be criticism of my HTML. Could I have added more semantic HTML? If so, where and why? If you're really hot and bothered feel free to rip my javascript apart but I know fully well how tiring that is when the project is not yours 😅.
Cheers!
Great job! I loved your solution, real cool!
Cheers!
P.S: I'm gonna use what u suggested for the filtering, I did it with a classical while loop I didn't remember about those two functions!
Feedback required
Hi uiguy,
Great solution to the challenge! Both the layout and the calculator logic are great.
Here some little suggestions:
max-width: 770px on the 600px media-query the calculator looks too stretched to my eyes.Btw, nice work with the theme toggle too.
Cheers!
HELP!
Guys, everything works fine, except for one thing that I'm trying to fix. If you take a look at the jobs languages and tools, you will see a comma between each item. Why is that happening? How can I fix this?
Happy coding :)
Hi Karim,
Nice solution to the challenge!
If you take a look at your html the data-filter attribute of some of the li.job elements have some commas, leading the JS to think the commas are also another language or tool? Idk that's my best guess
Cheers!
Please give some feedback about accessibility and design.
Hi Manan,
The solution looks good, here some improvements:
Rest of it looks nice, good work!
Cheers!
Constructive feedbacks are welcome : )
Hi Saif,
This is a great solution! I love the approach you took with CSS Grid, it's the same approach I took
Some suggestions:
In a 4k screen the cards look way to streched and it's difficult to read, that's mainly because you're setting a font-size: 62.5% on the html selector. I would suggest you define a 12px font-size in the html element with a max-width of 1400px in the .container element.
If you want to center it to the middle instead of using huge margins I would do some basic flexbox centering like this:
main{
min-height: 100vh;
display: flex;
justify-content: center;
}
Keep on the good work!
Cheers!
My main point of confusion is why is rem useful as a unit of measurement? I've seen a lot of other solutions using this in their projects and I have read up on the uses of it on my own. However, I can't seem to wrap my head around why the rem unit is useful. I'd be more than happy to learn more about it!
Hi bryan,
We use rems mostly for responsiveness, here an example of it:
font-size: 16px in the * selector and then we set a font-size for an h1 element like this: h1{font-size: 2rem}, this will give us a 32px h1 element. The problem now it would be that it'll look great in a laptop but in a mobile it may look a bit big, to fix this we'll use media queries and will set something like:@media (min-width: 768px){
*{
font-size: 12px;
}
}
That would be a good reason why you should use rems to set a font-size. If you want a more extend explanation about this you can go check this article where Kathleen McMahon dives deep into this subject.
Also: Your solution looks way to small in 4k screens mainly bc of the font-size set in the * selector
Cheers!
I still don't know how to use grid and flex box properly, so any recommendation for content alignment is welcome.
Hi Thiago,
First your solution looks good, here some improvements:
Although you got the layout correct and the responsiveness works fine, there is to much whitespace. To improve this I would remove the grid-template-rows, height & width of the .conteiner element. This way we let the content fill the width and height.
You should avoid defining font-size with pixels and instead use rems, what I usually do in this challenges is set a font-size with the * selector such as:
*{font-size: 16px;} , once this is done every time I have to define a font-size for an element let's say an h1 I'll do: h1{font-size: 2rem}. What this does is it takes the 16px defined with the * selector and multiplies it by 2 which results in 32px. This is just how the rem unit works, here and article explained with more details.
You should redo the Sign up button part where you use a a tag wrapped in a p element. For this you should use the button element and style it with a background and a box shadow to make it look like the design you are given. You can look on the internet different ways to style button elements with CSS.
To finish, in the why us part instead of using a p element and a bunch of <br> it would be better to use an html unordered list.
Keep on the good work!
Cheers!
Do you think I could optimize my javascrpt code? Let me notice.
Hi Matheus, your solution looks great! Nice work!
One way to optimize your JS would be by handling 99% of the theme toggle with CSS instead of manually changing the value of each element with javascript. So you could do this:
body such as theme-1, theme-2, theme-3.body.theme-1{
(theme-1 color variables here)
}
body.theme-2{
(same variable names but theme-2 color values)
}
body.theme-3{
(same variable names but theme-3 color values)
}
You can see my solution for a live example of this approach.
This is just optional and if you are happy with your approach then go on with it but I saw this and I wanted to let you know there is actually a very easy way to do it with CSS and lightweight JS script.
Anyway, nice solution and keep on the good work!!
Cheers!
I think HTML markup have some room for improvement but can't exactly point what i don't like about it. Any ideas ?
To be honest most of the time i spent on this project i worked on making background somewhat responsive and i'm preety proud of how it worked out? Did I messed up anything I can't see ?
Hi powerz,
Your solution looks fantastic! I don't really see any improvements for the markup.
However, I do suggest you increase the font-size of the footer, in very large screens it's too small to read. I would give it like a 1.2rem.
Rest of it is great, see that you use semantic html tags like main, section, ... Responsiveness is great too. Heck, I like your solution more than mine lol.
Keep on the good work!
Cheers!
Any feedback is appreciated.
Hi Anosh,
This solution looks great,
The only thing is that the idea of this challenge is that you have three cards that are equal except for the colors, so the best way will be to define the same class for the three cards and then using CSS's :nth-child() selector change the colors of each card. This way you are not writting the same CSS three times, it's cleaner code and you take half the time to write the code.
You can have a look at my solution to this challenge to get what I mean
Also, beware the responsiveness! In really small devices the cards get to small, you should put a min-width on the card elements to avoid it.
Cheers!
