Looking to hire developers?
@Da-vi-de
@Aadv1k
Submitted
@Da-vi-de
Posted
Hi, it's good to see you tried to make it work, this project was my first one and i thought it was easy but it's not at all. So, in order to make it work you need Javascript, JS is needed not only for that, the component is called "interactive" because it changes content when you move the bar! The slider should have a mouse event like so:
slider.addEventListener('mousemove', function() {
const activeSlider = slider.value;
const sliderColor = `linear-gradient(90deg, rgb(165, 243, 235) ${activeSlider}%,
rgb(234, 238, 251) ${activeSlider}%)`;
slider.style.background = sliderColor;
});
As you can see,
first you need to get the value, the value is an attribute that resides in your slider range in html file, like so <input type="range" min="0" max="100" value="50" step="25" >
i saw your code and you miss the step attribute which is important when you need to make the slider interactive.
then you need to set the color so that it changes when you move it and that will be the background.
Yes, the slider behave differently across browsers but recently i've found a nice article that teaches how to make a custom range input that looks consistent across all browsers! Here's the link: article link range input
If you are interested in knowing more you can see my solution where there's the most important JS part needed to make the component interactive! Hope it helps, keep coding!
Marked as helpful
@dkhenrique
Submitted
Hello devs, I would like an analysis about this challenge and about the source code that was developed for this project. I accept constructive criticism and tips on how to improve it, thanks for the advance.
@Da-vi-de
Posted
Hi, congrats, it's a really nice result for this challenge. Good use of semantic HTML. It's nearly perfect, i noticed the instagram card misses that peculiar angle. You need a pseudo element and positioning, i did it this way:
.instagram {
position: relative;
}
.instagram::before {
position: absolute;
content: '';
width: 100%;
height: 4px;
top: -1px;
left: 0;
background: var(--instagram-color;)
border-top-left-radius: 5px;
border-top-right-radius: 5px;
}
The cursor doesn't need to be pointer, it would be great if there was the hover effect on cards.
Good job, keep coding :-)
Marked as helpful
@nenamartinez
Submitted
First completed challenge! I seem to have some issues with padding when it comes to smaller sizes (~450 px or smaller), the screen seems to force a scroll and I'm not sure why. Advice on how to write more concise code and advice about responsive design would be particularly helpful. Thank you.
EDIT: Oct 22, 2021 - I re-uploaded the code with some semantic corrections as well as a fix to the responsive design. Thanks for the great feedback!
@Da-vi-de
Posted
Hi Elena, it's not a bad result for this challenge but it will be nice after making some changes in your code.
First thing, i'd recommend checking the report and try to resolve those issues. I try to guide you for better understanding:
The alt attribute must always be included in images (except decoratives images, the attribute can be left blank) tags.
The issues about landmarks is because your HTML is not semantic, that means it can't work properly with assistive technology. You should read about landmaks by clicking the link Learn more but i tell you what you miss in your code.
<main></main> element rght after the body tag, read about it here
The h2 heading needs to be a h1 instead! Heading elements should be in a sequentially-descending order, there always must be a h1 heading.
The attribution goes in <footer></footer> element.
The padding problem you referred is casued by a wrong approach, which is not mobile first. You used a media query that indicates a width smaller than 600px in which you applied just the padding but the rest of the code is for desktop, you need to resize your entire card for a small screen. This doesn't happen when the workflow is mobile first because you start working for the smaller device width, you can surf the web and you'll find tons of articles tutorial etc..
By the way, that little horizontal scroll bar is there because the card width is larger than the mobile width, the CSS property overflow controls the scrollbar behavior when it's set to hidden you don't see scrollbars anymore.
Hope it helps a little, keep coding :-)
Marked as helpful
@CaioRoman
Submitted
Hello, this is my first challenge, and i'm learning css and html, so any constructive criticism is welcome, p.s. sorry for my english, i'm brazilian and i'm still learning english so i'm using google translator
@Da-vi-de
Posted
Hi Caio, it's a nice result as first challenge, well done!
Good use of semantic HTML except at the beginning after body tag, the container shouldn't be section but main instead. section is not semantic and it's not the right region landmark for containing big code blocks.
On mobile there's a horizontal scrollbar, you can get rid of it by adding overflow: hidden to your body in CSS.
Keep coding :-)
Marked as helpful
@AlanBoulesteix
Submitted
Hey everyone !
With this projet, I realize that I wasn't good enough in responsive design.
So I'll reread doc for it, but if you have any site where I can learn some tips, feel free to send it !
Have a good one !
@Da-vi-de
Posted
Hi, i've been there so i know the struggle! I suggest you to follow one of the best instructor, Kevin Powell, here is the link to one of his free videos where he explains clearly the main concept of mobile first approach! You can start from there, for more insight about it or any other subject you can also follow FEM slack channels where you can learn more from the community!
@media all and (min-width: ...) { }
Keep in mind that you need to make a decent/good/as perfect as you can job for three main devices: mobile, tablet, desktop.Hope it helps a little, keep coding :-)
@ttakeyaya
Submitted
Any suggestions would be greatly appreciated.
@Da-vi-de
Posted
Hi, it's a very nice result for this challenge that looks super easy to implement but i rember it has tricky parts! Well done.
Keep coding :-)
Marked as helpful
@nayanabhatm
Submitted
@Da-vi-de
Posted
Hi Nayana, the overall result is nice, it looks good on desktop but i can't say the same for mobile screen.
Experimenting is always a great thing because it means you are curious and you want to understand deeply what you do. This is the kind of attitude that allow you to grow, well done :-)
Now i tell you what i learned (and i'm still learning) about positioning! Basically position absolute, relative, block are not longer in use when the main goal is to make something responsive, so a good question to ask to yourself (before starting the project) would be: Do the items need to be responsive? If so, then the best choice is to apply flexbox or grid technique! Rarely you need positioning items with float etc.. But it happens! The main focus though it must be responsiveness. The problem in your implementation is also the workflow, it's not mobile first! I suggest you to change approach, you will understand the benefit of doing so!
Honestly it's the first time i saw variables written that way, i'd rather write variables for --font-size. Usually larger projects have lots of different measures. Personally i wouldn't do the way you do, it's very time consuming.
Hope it helps a little, keep coding :-)
Marked as helpful
@frontendnus
Submitted
I'm done with this project but something confuss me. How to make the line or in my code is border-top is full width, not crash with the padding? Please give me your feedback
@Da-vi-de
Posted
Hi, in your case you could delete border-top, in HTML add a <hr> tag right here:
<p class="city">London</p>
<hr>
<div class="social">
then you can style it like so
hr {
padding-right: -3em;
padding-left: -3em;
border: 0 none;
height: 1px;
background-color: choose a very light grey color
color: choose a very light grey color
}
you have padding: 3em in class .content if you want an extended line you need to take that padding (left and right) off! It should work.
Try in your text editor the above code, hope it helps, keep coding :-)
@Tuason066
Submitted
Hi, I am a total beginner in programming can you please check my codes if I write it correctly or is it bad so I can improve it. Please critic my work. Thank you.
@Da-vi-de
Posted
Hi, great result on this challenge, full responsive; in my opinion you used correct semantic HTML elements.
<div></div> , <main></main> etc... Of course that's a small project but for a larger code base it's better when the code is legible! With that being said, well done!Keep up the good work :-)
Marked as helpful
@ravenloue
Submitted
I struggled the most with the background elements on this one. But once I figured it out, the rest was smooth.
Something I'm not certain about is how to fix the accessibility issues that the solutions page shows. The documentation provided when I visit the error is confusing to me. I've tried to fix it on other projects but just end up with more errors.
@Da-vi-de
Posted
Hi, it looks good, good result on this challenge! The root of the problem for what i see is the absence of semantic HTML, that's the reason you have those issues, i try to explain something.
<body> opening you have 2 <img> tags, starting right away like this it's not a good practice, you should open a <main></main> element after body and wrap all the code up to <footer> element, like so:<body>
<main>
all your code
</main>
<footer>
footer code
</footer>
</body>
The main element contains the card which is the only thing in the page, so it's the main content; the footer though is not part of the main content because <footer></footer> has a specific meaning in a web page, typically contains information about the author of the section, copyright data or links to related documents. I guess by now you're starting to understand what a navigational region is.
<div></div> elements which are used mostly for styling purpose! For example i saw the last part of the card is kinda messy in HTML, you wrapped all individual headings in divs whereas you could use an appropriate element like an <article></article> and group together what it needs to be together, like so:<article>
<h3>80k</h3>
<p>followers</p>
</article>
I saw you used <h1><h1> heading, i used <h3></h3> instead because h1 can't be reused and heading have a discending order in the page.
Hope it helps, keep coding :-)
Marked as helpful
@Ibkodus116
Submitted
Hello, please feel free to make corrections this is my first HTML & CSS project.... I really hope to get a feed back from y'all
@Da-vi-de
Posted
Hi, nice result on this challenge, it looks good either mobile and desktop with no media query. Well done!
font-size should be smaller and it should be position at the bottom edge of the page. I'd also recommend checking the report and try to resolve those issues, some of them are quite important for accessibility!Keep coding :-)
Marked as helpful
@angelostd
Submitted
I still thinking that I need help with media queries. (?)
@Da-vi-de
Posted
Hi, good result on this challenge. Well done!
min-width: ..., that's it! You wrote the last media query max-width: 900px in which there's the background-image, you could do it at the beginning, what you've done doesn't really make sense because it's like saying "make changes when the width is smaller and equal to 900px" but you already took care of that, so you can delete it and move that code in your first body. The first media query is right!Hope it helps a little! Keep coding :-)
Marked as helpful
@mizek1
Submitted
Feedbacks on how to improve the media queries and the code in general would be cool.
@Da-vi-de
Posted
Hi, it's a nice result but i would make the card for desktop version bigger , font-size seems a bit small at that width. Also if you want to become a pro in the future get used to separate HTML and CSS. The style should be in its own file style.css. Keep coding :-)
Marked as helpful
@TiredQuan
Submitted
I am wondering about how does chrome works compared to firefox, Because when I test my code I usually use firefox instead of chrome, the solution I made looks completely fine on firefox but it's off in mobile view chrome
@Da-vi-de
Posted
Hi, nice result on this challenge, well done. I tried on both Chrome and Firefox. No problem either browsers! They look the same.
media-query 5 times, just one time and put everything there, that's it!Happy coding :-)
Marked as helpful
@anhoang241998
Submitted
Hello everyone, I had just finish the challenge. Any comments or suggestions on how I could have done this challenge better or made it more responsive would be greatly appreciated. Thank you in advance.
@Da-vi-de
Posted
Hi, it's a good result but it could be perfect, there's just a problem with the responsiveness of the website.
First thing first i encourage you to change approach and start the mobile version, that's the professional and modern way of working. I believe you work with the dev tool set on responsive mode, just determine the width you want to work at, for example 350px, build your project and then add media queries min-width: ... trying to respect devices screen size!
I didn't do this challenge but i guess the image shouldn't be cropped because it's the desktop version, you change it too early, so i think it's better keeping the column and resize it for tablet width.
Hope it helps, keep coding :-)
Marked as helpful
@Briancarlo24
Submitted
Did I do this challenge right?
@Da-vi-de
Posted
Hi, good result on this challenge. Almost perfect!
min-width: 375px, it's too early making desktop design at that width. You should have tried a different strategy so that it would look good on a tablet as well. When there's no tablet design it means there's nothing to change nor to add! If i open the website at 768px (common tablet size) it doesn't show all the card properly, two of them are cut, because you already coded desktop design!Keep coding :-)
Marked as helpful
@Faerk77
Submitted
Hi!, Trying to keep working and doing my best. This is my third challenge.
I would like to know if I am catching bad programming habits. Or if I'm making stupid but avoidable mistakes early on.
I will appreciate any help.
Thanks!
@Da-vi-de
Posted
Hi, nice result on this challenge, well done!
HTML seems perfect, CSS not really! There's no need to repeat the media query over and over, there are two ways for writing media queries: Either you add them at end of each class or you add them at the end of the file. I'd rather write them at the end of the file, it's more organized, i don't get confused and i foucs much better at tasks!
I've also noticed you don't follow mobile first approach, i encourage you to do that, starting from small to large width screen, use min-width: ..., that was a very small project but you may go through very hard times for bigger designs!
Hope it helps a little, keep coding :-)
Marked as helpful
@annab6
Submitted
I haven't managed to make the border thinner than 1 px (seems impossible). Any feedback is very welcome!
@Da-vi-de
Posted
Hi, the card for desktop version seems a bit small but it's a good result on this challenge. Well done!
border property, yeah maybe it can't be managed too much. When i need a very thin line i use <hr> tag but be careful because it's a bit tricky, if you set margins or padding in a card you need negative margins (either left and right) so that you are able to extend it! Anyway, try this snippet of code in your text editor (don't change your code, the line looks good) it might comes in handy in the future.hr {
border: 0 none;
height: 1px;
background-color: choose a very light color;
color: same as background-color;
}
Happy coding :-)
Marked as helpful
@moein459
Submitted
I like to get your feedback.
@Da-vi-de
Posted
Hi, Nice result on this challenge, well done!
Keep coding :-)
Marked as helpful
@R3XCran1um
Submitted
Any feedback is appreciated, particularly about responsiveness. TY
@Da-vi-de
Posted
Hi, Nice result on this challenge, well done! You can run the report again and it should clear your issues, i saw you fixed them!
Keep coding :-)
Marked as helpful
@harryHerdiana
Submitted
I would like to know more about responsive design, how we specify the min or max media query when the requirement on the project (for example) is Mobile: 375px and Desktop: 1440px?
@Da-vi-de
Posted
Hi, you don't need to worry about the exact design size, it's not a requirement, it means that at that width your project should look like the design!
If you work with dev tool open (on responsive mode) in your favorite browser then start coding at 350px , although the smallest mobile screen width should be 320px. Once you've finished increase the width until you see the UI is changing too much, usually that happens at around 500px (tablet width), be aware that there are projects here on FEM with tablet design as well, in many of them you need to change or add things! You repeat the process for all the main breakpoints 768px, 900 and something and so on... This way of working is called mobile first.
When it comes to responsive design the hardest thing to do is making good development choices since the beginning, those choices can make the difference throughout the project.
Hope it helps a little, keep coding :-)
Marked as helpful
@Nick331102
Submitted
I'm looking for any general feedback about where I can improve this project. More specifically, I can't figure out why I cant get the error or success messages to show for the email validation in the form. Thanks
@Da-vi-de
Posted
Hi, nice result on this challenge, although it's not mobile first approach!
There's a typo in your HTML, the closing main tag misses /, also i'd recommend checking the report and try to resolve the issues.
In JS you add the class show but in CSS that class has display: block it should be display: unset because you set display: none to email__error, email__success but this isn't enough yet. First you need to remove the class in your else statement, you can't just add the class, you also need to remove it! And second either use an id for selecting emailSuccess and emailError or different class, styled classes shouldn't be used for selectig.
Hope it helps, happy coding :-)
Marked as helpful
@chrisyoung0101
Submitted
Used flexbox for the content (overview, internal structure, & surface geology buttons, planet image, & text). For tablet and desktop, how can I keep the content centered when changing size of screen?
In index.css, is there a better way to handle media queries? My approach seemed to make the css overly complicated.
Used javascript to handle the showing / hiding of content when clicking the overview, internal structure, & surface geology buttons. My solution ended up being a bit bloated. Is there a way to set up a function that could maybe loop through a list of properties that would handle this behavior?
Used js to change the background color for the overview, internal structure, & surface geology buttons when on a specific planet. How could I make this code simpler / less code?
Used js to change the border color for the overview, internal structure, & surface geology buttons in mobile when on a specific planet as well as changing the border color when on tablet / desktop in the nav menu (planet names). How could I make this code simpler / less code?
@Da-vi-de
Posted
Hi, that's a nice result for this challenge because it's done in plain JS and it's not easy as it seems to be. I get a sense you put all your effort to make it work and it works, so be proud of it! I can't judge all your code because there's a lot to process but i did the same challenge and at least i can answer to one question (which is important i guess) you asked.
eventListener to the parent element you can tell JS exactly the event.target which are the buttons in our case, the buttons have the attribute data the value is a number, when the code is interpreted it goes down to the end of the parent element (capturing phase) then like boiling water it goes all the way up (bubbling phase) and it stops when it gets to the beginning of the parent element where the event has been fired, so you take as many birds as you want with just one stone. I can copy and paste my code here but it's a lot, so feel free to go to my profile click on my project and inspect github code, take your time because it's not straight forward, especially JS. In case you need to clarify anything about it ask me.Hope it helps a little, keep coding :-)
Marked as helpful
@salahudheen007
Submitted
any suggestions to improve the code?
@Da-vi-de
Posted
Hi, i don't really like your choice of implementing CSS in style element, you should have a saperated CSS file, that's the professional way of working.
You didn't really follow the task, the background must cover the entire page, in practice means you need to selet the body element and set the background there.
id should be used for JS, ids are unique so you can't reuse it again, it's a problem for bigger projects because you end up writing everything twice or more...
Never underestimate reusability.
I strongly advice you to learn mobile first approach and HTML semantic, a right way to start would be checking your report and try to resolve the issues.
Keep coding :-)
Marked as helpful
