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.
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 :)
-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?
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 :)
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.
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 :)
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! 🙋♂️
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
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 :)
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.
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 :)
I will love to have a feedback on how I can make the toggle better
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");
})
})
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.
Hey Steven ! Great job on your solution :) Did you use the "developer's tools" feature on Chrome ?
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 :)
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 !
why my javascript code is not running :/
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 !
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?
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 !
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!
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 :)
