Looking to hire developers?
@MartineauRemi
@FaideJaures
Submitted
That was a tough one, I learn a lot about DOM manipulation. Despite a few small mistakes, I m pretty proud of it. But I need to learn more on how to make the javascript less messy. If you have advices or observations, write in the comment please.
@MartineauRemi
Posted
Hey ! Congratulations, you did a great job ! Here are some small improvments you could implement to make your JS file easier to read / udpate.
You could define your functions in a separated file, and import them in the main.js file. That way, you can see better which part of the code is applied on the page.
There are a lot of 'for loops' in your code, and sometimes, for loops inside of other for loops. This can create a really verbose code and it may be hard to read. You could use some functions from JS such as map (link to map documentation). For example, in your code (main.js - line 128) : You could replace this :
for (let i = 0; i < data.comments.length; i++) {
mainContent.innerHTML += commentObject(data.comments[i].user.username, data.comments[i].user.image.webp, data.comments[i].createdAt, data.comments[i].content, data.comments[i].score)
for (let j = 0; j < data.comments[i].replies.length; j++) {
let replySection = document.getElementsByClassName('replySection');
replySection[i].innerHTML += replyObject(data.comments[i].replies[j].user.username, data.comments[i].replies[j].user.image.webp, data.comments[i].replies[j].createdAt, data.comments[i].replies[j].content, data.comments[i].replies[j].score, data.comments[i].replies[j].replyingTo)
}
}
With this :
data.comments.map((comment, index) => {
mainContent.innerHTML += commentObject(comment.user.username, comment.user.image.webp, comment.createdAt, comment.content, comment.score);
comment.replies.map((reply) => {
let replySection = document.getElementsByClassName('replySection');
replySection[index] += replyObject(reply.username, reply.user.image.webp, reply.createdAt, reply.content, reply.score, reply.replyingTo);
});
});
Notice the parameters passed to commentObject() and replyObject(). It is overall, easier to understand in my opinion. (I used an arrow function inside map. If you're not familiar with this concept, here is the doc.)
function toLike() {
let plus = document.getElementsByClassName('plus');
let moins = document.getElementsByClassName('minus');
let screen = document.getElementsByClassName('screen');
for(let i = 0; i < screen.length; i++) {
plus[i].addEventListener('click', () => {
screen[i].innerHTML = parseInt(screen[i].innerHTML) + 1;
moins[i].disabled = false;
plus[i].disabled = true;
});
moins[i].addEventListener('click', () => {
if (plus[i].disabled) {
screen[i].innerHTML = parseInt(screen[i].innerHTML) - 1;
moins[i].disabled = true;
plus[i].disabled = false;
}
});
}
}
Hope this is helpful, keep up the good work ! Happy coding :)
Marked as helpful
@dominikapap
Submitted
-Did I use any bad practice? -Is using px ok here, or should I have changed them to rem? -What could have been done better?
@MartineauRemi
Posted
Hey Dominika ! Nice work on this solution, congratulations :) Here are some tips :
In your case, you could replace <section class="main"> with <main class="main">. Same thing with the <section> wrapping your attribution, at the bottom of your html file. Why not use a <footer> tag instead ? :)
For this particular example though, I would put something in the alt property, since the image is not purely decorative, and a description could be helpful for screenreaders users.
Hope this can help you ! Keep up the good work, and happy coding :)
Marked as helpful
@Nazemrap
Submitted
Got some trouble here and there, but in the end just one detail i couldn't fix is the alignment of the ETH and clock with there respective text... maybe i have played to much with some position/display things.
@MartineauRemi
Posted
Hey there ! Congrats on your solution, it's looking good :) If you want to align your icons with the text, do something like :
#clocks{
display: flex;
align-items: center;
}
.clockpic{
margin-right: 8px;
}
Just do the same for #ETH and you should be good to go. Here are a few leads for improvment:
.card{
padding: 24px;
//the rest of your code
}
(and you get rid of the unnecessary margins in all your elements). That way, you would have to change only this particular padding, and save a lot of time.
Hope this can help you. Keep up the good work ! Happy coding :)
Marked as helpful
Hi everyone! 👋
This was the first time I used an API for a project / challenge like this. It was not as hard to get it working as I thought it would be. However I am not sure if I did it the right way, because I am very new to this.
Also I tried to keep in mind that changing text on screen, like with this challenge, is hard to notice for people with screen-readers so I tried to use aria-live for that. I also added a focus to the button in CSS for better accessibility.
❓ Concrete questions ❓
📑 GitHub: https://github.com/royschrauwen/100-days-of-code/tree/master/day-21
💻 Live: https://royschrauwen.github.io/100-days-of-code/day-21/
I also did this project for the #100DaysOfCode Challenge! Today is day 21 and I am still very much enjoying it!
Have a nice day! 🙋♂️
@MartineauRemi
Posted
Hey Roy ! You did a nice job completing this challenge, congratulations :) The js part is good, but could be improved a bit :
You used Promises but didn't cover the eventuality of a failure coming from the API. So you could add a 'catch' clause to handle errors, and display a custom error message for example. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises
Your js file is small for now, but its size may increase over the time. I suggest you take the code inside of 'then' and create a separate function, that you call in your 'then'. Your code will be easier to read (especially for other developers working with you). This advice is purely personal though. Others will tell you there is no need to do that.
This one is a detail, but I think you forgot to remove a console.log line 28.
Hope this clear enough and that it can help you. Keep up the good work, and enjoy your #100daysOfCode challenge :D
@Shikhar0411
Submitted
@MartineauRemi
Posted
Hey !
You gave the width property a value of "30%" on your container. This means that the container will take 30% of the viewport, and this is the reason it is resizing along with the browser. If you want the width to stay the same no matter the size of the viewport, you could give it a fixed value (e.g. width: 340px;)
The given desktop and mobile widths are just indications on how the design should look like on those 2 particular values. If you open your dev tools tab in your favorite browser, you can actually target those particular widths and get a better idea on how your design should look like.
Hope this can help you ! Keep up the good work, and happy coding :)
Marked as helpful
@KhizarSa
Submitted
Hi guys, I have work hard on this project and i know i've written alot of inefficent code. I would love to get your guys feedback do checkout the website and code. Thank you.
@MartineauRemi
Posted
Hey ! You did a great work, and your solution is looking good ! You should wrap your social media icons with <li> tags in your footer section. Happy coding :)
Marked as helpful
@Captressketh001
Submitted
I will love to have a feedback on how I can make the toggle better
@MartineauRemi
Posted
Hey ! You did a great job :) Concerning the menu toggle, I think it should work with only this in your main.js file :
$(function() {
$(".toggler").on("click", function(){
$(".nav-items").toggleClass("active");
})
})
@srz2
Submitted
Current Known Issues, Help Would Be Appreciated
Generally, what is the best way to debug for mobile? When looking at the mobile design for the project, I tried to use chrome in a small window but only can match it closely, not exactly.
@MartineauRemi
Posted
Hey Steven ! Great job on your solution :) Did you use the "developer's tools" feature on Chrome ?
@Dawid7600
Submitted
I finished my second project on the Frontend Mentor website. However, I have 2 problems with my project:
I will be very grateful for your help with my problems as well as for tips and advice to be able to improve my skills :)
@MartineauRemi
Posted
Hey ! Congratulations, you did a great job :)
I see you used CSS Grid to build your layout. You can set the row gap value using the property 'row-gap' in your container.
One way you can deal with the background quote symbol is :
Hope that helps Happy Coding !
Marked as helpful
@mahnoork18
Submitted
why my javascript code is not running :/
@MartineauRemi
Posted
Hey !
For the button, you have to change the onclick attribute from "myFunction" to "myFunction();'
Since you created a variable called 'icon', I think you need to replace
social.style.display
with
icon.style.display
var icon = document.getElementsByClassName('social')[0];
or you can access it like this (probably better) :
var icon = document.querySelector('.social');
The second method basically means that you're getting the first element having the class 'social'.
Hope that helps :) Happy coding !
Marked as helpful
@Aklog-1
Submitted
If you guys see some bushing around on my java script and know some clever ways, please share them to me. And, one another thing, what can I do to refresh the page/stop preventing the default/ and actually submit it to back end if there are no errors?
@MartineauRemi
Posted
hey ! Congrats on your solution, you did a great job :)
In your javascript functions from script.js, when you want to return a boolean value, you don't need the 'if/else' blocks. So for example this :
function empty(field) {
if (field === "") {
return true;
}
return false;
}
does the same thing as this :
function empty(field){
return field === "";
}
but the second is a bit shorter and easier to read.
Happy coding !
Marked as helpful
@skochdev
Submitted
Hi friends, decided to test my knowledge and practice some challenges. I learn CSS for two weeks now, so I don't know much. Had a couple quirks in the pricing sections. I used absolute position way, which I heard is a bad practice, but I couldn't figure out how to make it work the other way.
It was really fun, thank you for the challenge, I will proceed with the next one. If you have any suggestions on how to make this code better, you are welcome to do so. Cheers!
@MartineauRemi
Posted
Hey ! Nice job, you did great :)
An alternative to absolute positioning could be using CSS Flexbox or CSS Grid. Both are very powerful and can help you build almost any layout you want. You can pick one and try to apply it in your project !
If you're interested and if you're not already familiar with it, here are two guides I found useful to learn both.
Hope that will help you, Happy coding :)
@DFO89
Submitted
How did I do on the desktop version? I had a hard time keeping the contents centered on the web page even though I used...
body { margin: auto; }
Also, I don't know why the main image isn't working since the href works everywhere else (codepen, VS Code). Sorry, very new to front end/github/ all the things.
@MartineauRemi
Posted
Hey ! This is a great start :)
If you want to center components in your layout, you could use Flexbox for example:
body{
width: 100vw;
height: 100vh;
display: flex;
justify-content: center;
align-items:center;
}
The width and height properties is for your site to match the viewport size. In case you need it, here is a great guide to learn more about flexbox : https://css-tricks.com/snippets/css/a-guide-to-flexbox/
You can also do it with CSS Grid:
body{
display: grid;
}
.container{
place-self: center;
}
Here is a guide to CSS Grid aswell: https://css-tricks.com/snippets/css/complete-guide-grid/
Flexbox and Grid are powerful tools to build any layout, I recommand you check it and to try them a bit ;)
For the image, it seems that your path in your html file is wrong. In your Github repository, the desktop image is in the 'stats-preview-card-component-main/images' folder. Try to change the src accordingly.
Hope that helps you ! Happy coding :)
Marked as helpful
@Numark12
Submitted
This is my first completed project. I would like to know some way to reduce lines of code in CSS (it was quite long for such a small page). Thanks!
Este es el primer proyecto que finalizo. Quisiera saber si hay alguna manera de reducir las líneas de código de CSS (quedó bastante largo para una página tan chica). Gracias!
@MartineauRemi
Posted
Hey ! Congrats on your first project ! You did a pretty good job :)
Here are some tips to reduce your css:
.card_mobile {
display: none;
}
.inner_mobile {
display: none;
}
.back_mobile {
display: none;
}
.imag_mobile {
display: none;
}
With something like this :
.card_mobile, .imag_mobile, .inner_mobile, .back_mobile {
display: none;
}
.attribution {
margin: auto;
margin-top: 150px;
font-size: 11px;
background-color: hsl(244, 38%, 16%);
width: 900px;
height: 350px;
display: flex;
border-radius: 10px;
}
Assuming you were working in a desktop-first approach, you dont have to copy all the properties in your other media queries, but only those who change. So in your '@media (max-with:600px)' you can change this:
.attribution {
margin: auto;
margin-top: 150px;
font-size: 11px;
background-color: hsl(244, 38%, 16%);
width: 250px;
height: 600Px;
display: flex;
align-content: flex-start;
border-radius: 10px;
flex-wrap: wrap;
}
With this:
.attribution {
width: 250px;
height: 600Px;
align-content: flex-start;
flex-wrap: wrap;
}
and keep all the code in the '@media (min-width: 600px)' but remove the media query itself. Try to find and delete the redundant code.
Keep up the good work, and happy coding ;)
Marked as helpful
@no7sag
Submitted
Hello! This is my first challenge. I've been learning to code and web design since a few months. Doing something responsive is not my thing yet. 375px and 1440px width as required in this challenge are looking good, though others sizes not quite. Any suggestions? Thanks in advance.
@MartineauRemi
Posted
Hey no7sag ! Well done so far, this looks like a great start ! I am still learning front-end development aswell, but here a few tips i hope you'll find useful :)
When it comes to responsive design, you should keep in mind those 3 intervals:
You can start by using 3 media queries, one for each interval.
About the tablet design (768px to 1439px), you could keep the image on top of the text as in the mobile design, and adapt the text layout the same way as the desktop design.
Also, if you want your website to take the whole window, you could do something like that in your css file :
body{
width: 100vw;
height: 100vh;
}
Anyway, keep up the good work ! :)
Marked as helpful