-This is my first project using API calls, I would like some feedback on my JavaScript so that I know if my code is efficient or not. -I am still struggling a bit on my media queries for mobile. I know to take a mobile approach first and any advice is welcomed.
Hi, Jonfernando - Your project looks great! And your API call is clear and easy to read.
You mentioned media queries — if you're taking a mobile first approach (meaning all of your CSS above the media query is your mobile design), then I think you want to switch your media query to read: min-width: 600px. So all of your styling applies to a smaller screen, and then as soon as the viewport hits 600px or wider, the styles wrapped in the media query will apply.
One other thing you might consider — right now, the only way to get a new bit of advice is to hard refresh the whole page. If you add a click event listener to the big green button, you can re-call the API when a user clicks it.
// wrap your fetch() call in a function
function generateAdvice () {
// API call goes here
}
// call it once on page load
generateAdvice();
// call it again whenever the button is clicked
bigGreenBtn.addEventListener('click', generateAdvice);
I hope this all makes sense. Feel free to reply back if anything is unclear.
What is the best practice for making rating? I used a list but not sure if there is a more efficient way to do it
Hey, Matthew - Great question! I think for something like this, radio buttons are your best bet. You give them all the same name, and the browser will automatically ensure the user can only pick one at a time. And since they're an official browser input, you can wrap them and your button in <form> tags and just have the one event listener on submit. You get a bunch of accessibility features for free doing it this way.
I would be grateful if anyone could tell me how I can write this in less lines and get rid of some of IF statements in my code.
Hi, Mahdi - Firstly, this interactive scroll you created is quite impressive. That's a lot of logic to figure out! I'm certain that building it this way was not a waste of time because you probably learned a ton.
One way to shorten your code (and make your app more accessible) is to leverage functionality that the browser gives you for free. In this case, instead of using a <div>, I might consider using the native <input type='range'> as your interactive scroll.
Your HTML would look something like this:
<label for='scroll'>
// 'min' and 'max' set because you have 5 possible values
// 'step' will cause the scroll to snap from value to value
<input type='range' id='scroll' min='0' max='4' step='1'>
</label>
And then in your JS:
// you can create an array that holds all your pricing options
const PRICING = [
{ pricePerMonth: 8, pricePerYear: 72, pageviews: '10K' },
{ pricePerMonth: 12, pricePerYear: 108, pageviews: '50K' },
...etcetera
];
// select your range input and listen for changes
const scroll = document.querySelector('input[type=range]');
scroll.addEventListener('change', () => {
// set your various innerTexts using your
// PRICING array and range input values
PRICING[scroll.value].pricePerMonth
PRICING[scroll.value].pricePerYear
PRICING[scroll.value].pageviews
});
// you could also add an event listener to your checkbox,
// and update your pricing each time that value changes
I hope this makes sense! I see you're already using a checkbox and a button, so you're no stranger to using native browser inputs. Please feel free to reply back if anything I wrote is unclear. Have fun!
i didn't make a changing theme anyone with an idea can help me please.
Hi, Idriss - Nice job here! Since you implemented all of your colors as CSS variables, it will actually be pretty easy to add the theme change.
I see you started with the dark mode, which is great. You'll want to add your background image(s) here, as well.
:root {
--bg-primary-clr: hsl(235, 21%, 11%);
--bg-secodary-clr: hsl(0, 0%, 98%);
--bg-third-clr: hsl(235, 24%, 19%);
etcetera...
--bg-image: url(./assets/images/bg-desktop-dark.png);
}
Next, you'll want to define a class for the light mode and re-assign your variables to their new respective values.
.light-mode {
--bg-primary-clr: NEW-COLOR-HERE;
--bg-secodary-clr: NEW-COLOR-HERE;
--bg-third-clr: NEW-COLOR-HERE;
etcetera...
--bg-image: url(./assets/images/bg-desktop-light.png);
}
And lastly, back in your JS, you'll need to make it so that when users click the sun icon, the light-mode class is added to the <body> element. I see you already have the moon icon in your header with a hidden class on it, so that will be easy to swap. And then of course, when users click the moon, you'll need to do the reverse.
I hope this makes sense! Feel free to let me know if anything is unclear. :)
Hello everyone, here's my solution for this challenge. I had a lot of fun building it and starting to get the hang of JavaScript, but still a long way to go. Though I got most of the logic behind the calculator, there's a an issue that I can't seem to figure out. For example someone born on the 15th of September 1977, I am getting that they would be 44years, 21 months, and 10days instead of 44years, 10months and 10 days. Your comments and feedback would be very much welcome on how I could solve this. Thank you very much.
Hey, Malick - I think the error might be caused by a backwards greater than sign on line 48 of your .js file.
It’s currently written as if (currentDate.getMonth() < birthDate.getMonth()) { ageMonths += 12; }, but I think you’re looking for inputs where the birthday has already occurred this year.
I hope this helps! I found this challenge difficult and enjoyed reading through your solution.
Another challenge tackling HTML/CSS/Vanille JS. First time ever using API. This was fun and I learned a lot.
Got some more tailwind practice with this project as well. Some colors are off but thats because I was using default tailwind libraries and not specific CSS colors. I was not worried about those details as much as I was about learning how to use APIs with JS.
Not sure why i'm struggling so much with layering elements using Z-index. I feel like I layer it properly then suddenly the layered element breaks with any sizing. I can't seem to layer things without using absolute. If I use relative it never breaks outside of the box its in. Maybe I should add a parent container? Not sure how to approach this problem with layering stuff like the button in this challenge. Will update, this was my first submission.
Any feedback is welcome
Hey, Cesar - I also get a little flummoxed by the position property. I don't know much about Tailwind, but with plain CSS — the thing you want you want to stick in place (the button) needs to be position: absolute, and the container you want to stick it to (the advice box) needs to be position: relative.
And then to get it to hang off the bottom of the box like it does in the design, you want to give the button a bottom of negative half the height of itself. So something like this:
container {
position: relative;
}
button {
position: absolute;
bottom: -30px; /* assumes the button is 60px tall */
}
That bottom: -30px; is saying, "stick to the bottom of the box, but move down 30px." A positive number would move it upward. So long as the button stays the same size, you shouldn't have problems with it moving around. I hope this helps!
The JS I used to calculate the age is mostly accurate, though in some cases it's off by 1 day (e.g. 21-04-1992). I'd be curious if anyone could spot the bug that causes this and tell me how to fix it.
Hey - this challenge really took me to task, it was fun to read-thru your completely different approach. I might have spotted the issue that's throwing your result off by a day.
In this checkIfLeapYear(); function, it looks like you're checking if next year (2024) is a leap year, rather than the year after the inputted birthdate. And since next year happens to be a leap year, you might be subtracting an extra day when this function runs.
if (checkIfLeapYear(d2Year + 1) && sDays + eDays >= 366) {
remDays = sDays + eDays - 366;
d2Year++;
} else if (!checkIfLeapYear(d2Year + 1) && sDays + eDays >= 365) {
remDays = sDays + eDays - 365;
d2Year++;
} else {
remDays = sDays + eDays;
}
Hope this helps! - E
Edit: Thanks to suggestions from @elyyyse I was able to resolve the issue...
Hi community! This is an incomplete project but I wanted to go ahead and submit it to get some feedback. This is my first project using Javascript.
For the most part I got everything to work, however I was unable to get the message that shows after the user clicks submit, to update according to the score selected. My populateScore function runs, but the score variable within it only recognizes that element that originally has the "selected" class. Even if it updates in the html file, which I can see in real time using the browser's developer tools.
Any help on what I am missing or doing wrong would be greatly appreciated!
Hey, CJ - You're so close! If you run your populateScore(); function inside of your submitButton event listener, it should work. The way you have it running, score = document.querySelector(".number.selected").textContent; is looking for an element that's already hidden.
Hi all! I'm sharing my solution to the interactive rating component, this was a really fun task to figure out for my first attempt at a JavaScript challenge on Frontend Mentor.
I'm still relatively new to JavaScript so if anyone has any suggestions on how to better format my program or any improvements that can be made please let me know.
Many thanks!
Hey, Kyle - I love how organized your code is, I should really get better at including comments like that. As far as improvements go, I believe the best practice is to build your <form> — including radio inputs and submit button — in your HTML markup. And then simply grab the input's value and update the 'thank you' text with Javascript.
That said, your approach was clearly intentional — I'm curious to know why you decided to create your form elements with JS?
The slip_id comes as undefined for some reason. I've many years without coding so, if some could help would be great.
Cheers from Ecuador!
Hey, Aaron - This looks great! Looks like you just have a small syntax error. This is what you get back from the Advice API:
{"slip": { "id": 28, "advice": "When you're looking up at birds flying overhead, keep your mouth closed."}}
So anywhere that you reference .slip_id in your code, change it to .id and that should fix the problem.
I have two questions:
The star SVG appears to have its tips cut off a bit once the border radius was added to it. How can this be corrected?
My main card element (with the ".card" class) for some reason won't respond to the width measurement in its CSS, but will respond to its max-width. Why? Am I doing something wrong or applying things weirdly?
Any feedback on my general coding is also appreciated, especially if you have any guidance around best practices etc.
Hey, @RichusD - Nice job with the transitions, that was a nice little something extra. On your two questions:
<div> and styling that. So something like:<div id="star-design">
<img src="images/icon-star.svg">
</div>
#star-design {
// styles go here
}
Hey, Nawal - Nice work! I made a pull request here, so you can see the few lines I added to grab the rating value and present it on the next card.
Basically, I selected the <div class="rating"> on the Thank You card and named it confMsg. Then in your change() function, I assigned the value of selectedDiv to a variable and updated the confMsg with that. This solution breaks some of your styling, but hopefully it gets the point across.
Next time, consider using <label> tags instead of <div>s for your <input>s. That way, even when you hide the default styling with display: none;, the <input> itself still gets selected when a user clicks on the styled <label>. Then you can take advantage of native functionality and use something like document.querySelector('input[type="checkbox"]:checked') to get the value. (I would also recommend radio buttons over checkboxes for this particular use case.)
I hope this makes sense. Feel free to reply back with questions!
