Paradox
@TarestaAll comments
- P@wraith-wallSubmitted 14 days ago
- P@wraith-wallSubmitted 14 days ago@TarestaPosted 13 days ago
I liked your work. Great job! Learned quite a lot after reading your code. I was just using the length as a criterion for calculating the strength of the password, but your approach to determining the strength by using the character types and the length is much more reasonable. Again, good work and all the best for your future projects.
Marked as helpful0 - @shadowbanksSubmitted 30 days agoWhat specific areas of your project would you like help with?
I'm open to any other suggestions.
Thank you for your time :))
@TarestaPosted 29 days agoHello, it is a great work that you have done. I learned a lot from your project. As I was testing your webpage, I noticed some logic errors that I can share with you.
- When we arrive at the website, and enter 0 for the bill or just select a tip button, the error "Can't be zero" shows up for the people field, which logically does not look right. For bill, the error should be above the bill field, and it should not show up when we have just selected the tip and have done nothing else.
- When I have performed the calculations once and hit reset. If I press any of the tip buttons, without entering the bill or number of people, I still get a summary of the total, which should not show up just yet because no bill or people number has been added.
I guess it would be much better if we refactor the code a bit and the bigger functions into smaller units so that each does just one job.
const calculateTip = () => { if (people === 0) { errMsg.classList.add("active"); return; } errMsg.classList.remove("active"); const tip = (billAmt * tipPercent) / 100; const tipPerPerson = tip / people; const totalPerPerson = billAmt / people + tipPerPerson; // console.log(tipPerPerson, totalPerPerson); tipAmount.textContent = `$${tipPerPerson.toFixed(2)}`; totalAmount.textContent = `$${totalPerPerson.toFixed(2)}`; }; function manageError() {} function calculateTip() {} function displayResult() {}
A separate function could be created to deal with just the error message. And, the calculateTip function as its name suggests just deals with the calculation of the tip and a third function could be created to deal with the UI that sets the tipAmount and the totalAmount. This makes testing easier and we can deal better with errors as they are limited to the scope of their function.
In the reset logic, you can add the logic to make billAmt, people and tipPercent = 0 so that problem number 2 that I mentioned above does not appear.
const reset = () => { removeActiveButton(); bill.value = ""; peopleCount.value = ""; customTip.value = ""; billAmt = 0; people = 0; tipPercent = 0; tipAmount.textContent = "$0.00"; totalAmount.textContent = "$0.00"; };
I apologise if I missed something here. I would be more than happy to clarify any part that might have confused you. It was a great effort on your part and I wish you all the best for your future projects. Happy Coding!
Marked as helpful0 - @superozzySubmitted over 2 years ago@TarestaPosted about 1 month ago
Hello, great job there. The only thing that I can suggest for json is that maybe we can try to reduce some redundancy that exists in your javascript code. Instead of calling the fetch function for every button click, we can create a use the dailyReport function to handle the data that is displayed and maybe call that whenever the button is clicked. Here is a bit of an idea.
function dailyReport(timeframe) { fetch(url) .then(response => response.json()) .then(data => { current.forEach((element, index) => { const timeData = data[index].timeframes[timeframe]; if (timeData) { // Check if data exists element.innerHTML = timeData.current + 'hrs'; prev[index].innerHTML = getPreviousLabel(timeframe) + timeData.previous + 'hrs'; } else { console.error(`Data not found for timeframe: ${timeframe} and index: ${index}`); element.innerHTML = "Data Not Available"; prev[index].innerHTML = ""; } }); }) .catch(error => { // ... (error handling as before) }); } daily.addEventListener('click', () => dailyReport('daily')); monthly.addEventListener('click', () => dailyReport('monthly')); weekly.addEventListener('click', () => dailyReport('weekly')); window.addEventListener('load', () => dailyReport('weekly'));
Otherwise, it was all god. Job well done!
0 - @Tonny-Blair-DanielSubmitted about 2 months ago@TarestaPosted about 2 months ago
Hi there, it was a great effort. The style for the bigger screens is almost identical to the design. However, the form, currently, does not provide email validation. I input several wrong address types and they were accepted as well. In your javascript, maybe you can add the functions for such types of situations. As I can see you are using browser in-built email validation, so you do not have to do much. Here is what I can suggest.
form.addEventListener('submit', () => { if (email.validity.valid) { //Show the thank you message } else { //Show the error message //Prevent the toggling of the screen to reveal the thank you message } });
Another thing is that the design is currently not working for screens smaller than 768px, so this can be another area that you might look into. Other than that it was a good job. All the best wishes for your future projects.
0 - P@DocForLoopSubmitted 2 months agoWhat specific areas of your project would you like help with?
All feedback is welcome!
@TarestaPosted 2 months agoYour code is excellent. I am still learning SASS so I will focus on Javascript here, which by the way was impressive. I learned a lot by reading through your code. When I did this project myself I had unintentionally added a lot of redundancy and someone was kind enough to point that to me. Maybe it could help in making your code a bit simpler too. So, here is how I think you can get rid of some of the extra code.
//Create a toggle function to toggle classList const toggleClasses = () => { shareOptions.classList.toggle("show"); if (isMobile()) { footer.classList.toggle("hide"); } }; //Create a remove function to remove the class List const removeClasses = () => { shareOptions.classList.remove("show"); footer.classList.remove("hide"); }; button.addEventListener("click", toggleClasses); activeButton.addEventListener("click", toggleClasses); window.addEventListener("resize", removeClasses);
Marked as helpful0 - P@RetroApeSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Figuring out SCSS and formula to use for clamp. With this, you can see how font sizes go from big to small, smoothly, without sudden jumps. I think it is a much better way than using
@media
, although it also has its uses, of course.This formula is already explained in README.md file.
What challenges did you encounter, and how did you overcome them?SCSS variables are not meant to be in
:root
. I found that out the hard way... actually reading about it... the horror.How to have a background image? You watch a tutorial, and then you do it your own way. Yeah...
@TarestaPosted 4 months agoI loved your solution, especially using the clamp for the responsive size across different screen widths. I read the explanation in your README file. The equations are well explained and you also provided an example. Thanks for that. I had always set up the middle value randomly, something of an average between the highest and lowest, but now I know how to calculate it properly.
The approach of making each section responsive across all screen sizes before moving on to the next one is also well-thought out. Keep up the great work and all the best for your future challenges.
1 - @LapupehSubmitted 4 months ago@TarestaPosted 4 months ago
Well, you have done a great job. I truly learnt a lot from reading your code. It was almost the same as the end design. I see that after 768px you reset the grid structure and let the browser decide on the number of columns and rows. This is a great approach. We can achieve the same result using Flexbox too. There was this property
grid-template-rows: repeat(auto, auto );
. You can eliminate it if you want. The rows would be generated automatically anyway and I think repeat does not accept auto as a value, so this results in an error too. So, yes good job again and all the best for your future projects.0 - P@josenegrete123Submitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Most proud of using both CSS and Flexbox.
What challenges did you encounter, and how did you overcome them?Some challenges where just figuring out spacing,
What specific areas of your project would you like help with?padding
andmargin
. Trial and error helped me overcome that.If there's a better way of writing the
HTML
or if this is fine.@TarestaPosted 4 months agoYou did a great job. Your final version is almost identical to the design. I learned quite a lot from your code. I am not an expert, so I can not say for sure, but your HTML structure looks good and well-organised. I loved how you used Flexbox for the smaller screen and switched it to a grid structure for larger screen sizes making good use of these layout algorithms. Keep up the good work.
Marked as helpful0 - @velvet-jediSubmitted 5 months ago@TarestaPosted 5 months ago
Hi there, that was some good work. I really liked how you used flexbox for the alignment of most of the elements in your design. This makes your page a whole lot responsive. Everything was great. Just something to take notice is that on smaller screens, the product-card does not have any top margin, so that might be something you can take a look into. Good job and all the best for your future projects.
Marked as helpful0 - @ntshpawarSubmitted 12 months ago@TarestaPosted 5 months ago
It looks very good. Good job with the responsive design. I have not learned React yet so I will not be able to provide any feedback on that, but your overall webpage looks great. There was just one issue that I encountered while I was trying to check the responsiveness, the image of the omelette disappeared between 576px and 768px. On inspection, I found that the following rule is coming into play. .d-none { display: none!important; } I am sorry as I said I do not know React or Bootstrap well, so I am not sure about the solution. Once you have managed to solve it, do let me know about the issue too. All the best and great work.
0 - @StonyDigiSubmitted about 1 year ago@TarestaPosted 5 months ago
Hello. You did a great job. Nice work with bootstrap. I had also previously used buttons for the various social links but switched them to anchor elements because I learnt that they are semantically more sound in this case. A button could be used in form submission or actions in a page, since these links could be used for external navigation, anchor elements might serve better in this case. Separating CSS from HTML could help in keeping code more manageable for bigger projects. Overall, you have done very well.
0 - @Stryde2022Submitted 5 months agoWhat are you most proud of, and what would you do differently next time?
to be honest, i still waste a lot of time so i am not happy
What challenges did you encounter, and how did you overcome them?applying the coloring
What specific areas of your project would you like help with?how to spend less time and do things the right and more effective way
@TarestaPosted 5 months agoGreat Job! I can relate that in the beginning, we can feel like we are spending a lot of time completing tasks that seem very simple. I do not have any great advice but let us just try to tackle one thing at a time. If you find a solution to something challenging, noting it down can be helpful in the future. Your colouring looks good. I am more comfortable using Hex rather than HSL. If you find it difficult to get the right colours maybe you can try the colours directly in the developers' panel to get a feel of how they would look when written in the actual code. If you are really concerned about the time. Maybe you can try focusing on one thing at a time, start with writing just the HTML first, and then move on to styling. Having the styles in a separate file also helps in keeping the code separate and easier to maintain. You can use the root element to reuse the common styles as variables (https://www.digitalocean.com/community/tutorials/css-root-pseudo-class).
Now, I might have said too much and I do not know if any of it will help. Just remember as you will practice more projects in the future, your efficiency will build up on its own. All the best and good work again.
Marked as helpful1 - @ad0v0Submitted 5 months ago@TarestaPosted 5 months ago
You have done a good job. I believe there are no issues with the overall project and everything looks well-structured. I just did not see any CSS for the mobile design in the style.css file. You might want to consider adding that for a better responsive layout. Just one more thing that you could perhaps work on is adding a bit more to the documentation in your README file so anyone visiting your site could understand quickly the summary of the overall project by quickly glancing through it. Good job overall.
1