I have added a little animation to the dice button. The only problem I have encountered is that when button is clicked twice too fast, fetch returns the same response. I am new to this and I am wondering if there is a way to improve this.
Heya Jan!
The animation effect you've added is super neat. Well done! π
The reason why you're receiving the same response is likely because of the Advice Slip JSON API itself. From the docs: "Note: Advice is cached for 2 seconds. Any repeat-request within 2 seconds will return the same piece of advice."
What you can do instead is try adding { cache: 'no-store' } to your fetch request. I've seen this as a suggested solution as well. Hope this helps!
Most difficult thing do create was to do (and I failed) change color on hover on buttons and letters inside buttons. I think I am missing out something here. It shouldn't be that complicated, I think. :D
Hiya Dean! Great job! π
You're right about it not having to be too complicated. Here's how to simplify things a bit. First off, I noticed you used id's, you can use classes instead. The logic here is:
button selector, or make a class that you'll use for all three buttons. (Note: in this explanation, I'll use the button selector instead of a class, but you can definitely have two or more classes for an element).border: 2px solid #fff;button:hover {
background: transparent;
color: #fff;
}
button-sedan, button-suv, etc. Then for each class, set the colour accordingly.That should help reduce/simplify the amount of CSS you've written. Hope you find this helpful!
How do I make it responsive to smaller screen size.
Hiya! Great job working on this challenge! π
The reason why your QR code component (<div class="border">) shrinks significantly on smaller screen sizes is because you've set the width to 30%. Here's a suggestion you can try to fix this:
.border {
// your other CSS code
width: 100%;
max-width: 320px;
margin: 50px 20px;
}
max-width makes it so that your .border div doesn't get any larger than 320px. I also added 20px margins on both the left and right side of your div so there's still a bit of space around it on smaller screens.
Hope this helped!
Finally i managed to make the hamburger menu working! Happy with my progress.
Heya Marco! I know responsive nav bars aren't always the easiest, so great job on this challenge! π
I checked out your code and this is what I've done for the menu.
checkSize and the useEffect hook to toggle your menu, you can use CSS classes. In your media query in NavBar.css, set nav-links to display: none, but also create another class called active with display: flex as a declaration. Something like this: .nav-links {
// your other CSS declarations
display: none;
}
.active {
display: flex;
}
The idea here is that when you click on the hamburger menu icon, you want to toggle some piece of state which should then set or not set the active class.
NavBar.css, set it to display: none on larger screen sizes and display: block on smaller screen sizes. This way the hamburger icon only shows up on smaller screens.NavBar.jsx, set showMenu's default state to false and then make an event handler handleMobileMenu that toggles the showMenu state value. Then add an onClick event handler to your hamburger icon div and pass handleMobileMenu to it. It'll look something like this: const handleMobileMenu = () => {
setShowMenu(!showMenu);
};
<div className="hamburger" onClick={handleMobileMenu}>
// your icon
</div>
NavLinks into NavBar.jsx instead of NormalMenu, then revised your code, so the end product looks like this (which I'll explain a bit below): <div>
<nav className="nav-container">
<div className="logo">
<img src={logo} alt={logo}></img>
</div>
<NavLinks showMenu={showMenu} />
<div className="hamburger" onClick={handleMobileMenu}>
<img src={iconMenu} alt={iconMenu}></img>
</div>
</nav>
</div>
showMenu to your NavLinks component. You'll use this to toggle the active class you made earlier.NavLinks component, toggle the class name like so:<div className={`nav-links ${showMenu ? 'active' : ''}`}>
// your code
</div>
This should sort out your issue for the most part, but for positioning and stuff, I'll leave that to you. Sorry this explanation got super long! If it's not very clear and you have any questions, feel free to reach out to me. Hope this helped!
What I found difficult while building the project was the css portion. The spacing was a little hard for me to get accurate.
Other than the css, I was pretty aware of what to do with my code.
I would love to know best practices when ensuring my components are in their appropriate divs and I would like to have some resources that can help me with my css knowledge.
Heya Joshua! This solution looks great! π
I've got a couple of suggestions, but I'll try to keep em' brief:
attribution div out of the container div. Understandably, this'll look a little strange at first because you've got body set to display: flex. To sort this out, set the flex-direction to column and the attribution div should sit right below your QR container div. Once that's sorted, you could add a bottom margin to your container div to add a tiny bit of spacing between the two elements.container div's sizing by setting a max-width. I had a look at the Figma files and it looks to be around 320px. Your container div should look something like this:.container {
width: 100%;
// alternatively, you could use vw units, so you could do something like:
// width: 90vw; (or any value you think looks best) instead
max-width: 320px;
padding-bottom: 2.5rem;
margin-bottom: 1rem;
}
With this set, you can remove the media-query blocks for your container div. I think that should help clean up your CSS a tiny bit.
As for CSS resources, I highly recommend Kevin Powell's Youtube Channel. He's got a whole range of videos about responsive design and getting started with CSS.
Hope this helped!
Any feedback regarding best practices on React and Styled Components are very welcome. Also, you may notice that my styled components are redundant, especially on background-colors. Is there a way I can set bg color once, and not have to repeat for every single child elements? Thank you in advance!
Hiya Ryan! Great job solving this challenge. It looks really well done.
About your background colours. I noticed that you had this in index.css:
* {
margin: 0;
padding: 0;
background-color: hsl(28, 62%, 91%);
font-family: 'Overpass', sans-serif;
}
The * selector selects all your elements, so it's added a cream background colour to all your elements. You'll want to remove the background-color declaration from that block above and then instead, set your body background colour to that colour (you can do this in your index.css file). Once that's sorted, you should be able to remove all the redundant background-color declarations in your child elements/components.
Oh, and another thing. I noticed that you had CSS variables defined in index.css. You should be able to use this across your other components as well. It might be a little easier to set your colours this way instead of using the hsl() values for each element.
Hope this helped! Best of luck with other challenges! π
My 1 accessibility issue is a warning that says "Links must have discernible text". The link in question is actually an icon (with an <i> tag), which is wrapped by an anchor tag. Is there any way to satisfy this requirement while not really changing the whole "Icon link" thing, or do I need to add some text or something? Thank you for any insight or solutions you can offer!
Other than that, this project was easy peasy for me. It feels good to stretch my brain and practice basic HTML/CSS every now and then! π
Heya!
You can add the aria-label attribute (and a value) to your <a> tag and that should sort things out. So it'd probably be something like: <a href="https://github.com/Risclover" target="_blank" aria-label="Github"><i class="devicon-github-original"></i></a>.
Everything else looks pretty great, well done! β¨
Most of my problems i had with the advices generated from API, because it doesn`t work on Firefox(it works fine on Chrome and Edge). If you have time please tell my what could be the cause of this problem. Any other comments will be also helpful.
Hiya! Great work solving this challenge! π
So I think it's a cache issue or something. Anyway, I added: { cache: 'no-store' } to your fetch method, so it becomes:
const response = await fetch('https://api.adviceslip.com/advice', {
cache: 'no-store',
});
This seems to solve the issue. Hope this helps and best of luck with future challenges!
Is there a way to dynamically change classNames in react onClick? I used state for this, which seemed unnecessarily cumbersome, see below code (in components/MainCard.js):
//display "active" state of timeframe dynamically
const [dailyIsActive, setDailyIsActive] = useState(true);
const [weeklyIsActive, setWeeklyIsActive] = useState(false);
const [monthlyIsActive, setMonthlyIsActive] = useState(false);
const timeFrames = [
{ id: 1, type: "Daily", active: dailyIsActive },
{ id: 2, type: "Weekly", active: weeklyIsActive },
{ id: 3, type: "Monthly", active: monthlyIsActive },
];
const clickHandler = (event) => {
const clickedTimeframe = event.target.innerText.toLowerCase();
switch (clickedTimeframe) {
case "daily":
setDailyIsActive(true);
setWeeklyIsActive(false);
setMonthlyIsActive(false);
break;
case "weekly":
setWeeklyIsActive(true);
setDailyIsActive(false);
setMonthlyIsActive(false);
break;
case "monthly":
setMonthlyIsActive(true);
setDailyIsActive(false);
setWeeklyIsActive(false);
break;
default:
setDailyIsActive(true);
setWeeklyIsActive(false);
setMonthlyIsActive(false);
break;
}
};
<p
onClick={clickHandler.bind(this)}
key={timeFrame.id}
className={timeFrame.active ? `${styles.active}` : ""}
>
{timeFrame.type}
</p>
Heya! So I tried tinkering a bit and this is the solution I've come up with:
In your MainCard component:
const [timeframe, setTimeframe] = useState('Daily');data-timeframe to your p element. It takes timeFrame.type as a value.clickHandler, I've removed the switch statement block and replaced it with: setTimeframe(event.target.getAttribute('data-timeframe'));className={timeFrame.type === timeframe ? `${styles.active}` : ''}To sum it up, here's what's changed:
const [timeframe, setTimeframe] = useState('Daily');
// I cleaned up your timeframes array a bit and just removed the active property since I didn't need to use it
const timeFrames = [
{ id: 1, type: 'Daily' },
{ id: 2, type: 'Weekly' },
{ id: 3, type: 'Monthly' },
];
const clickHandler = (event) => {
const clickedTimeframe = event.target.innerText.toLowerCase();
setTimeframe(event.target.getAttribute('data-timeframe'));
props.onChangeTimeframe(clickedTimeframe);
};
and
<p
onClick={clickHandler.bind(this)}
key={timeFrame.id}
data-timeframe={timeFrame.type}
className={timeFrame.type === timeframe ? `${styles.active}` : ''}
>
{timeFrame.type}
</p>
Not entirely sure if this is the most optimal way to solve this, but it's a start. Hope you find it helpful! By the way, your solution looks great! π
Do you think i have added unnecessary CSS? Suggest some best practices to make page more responsive?
Heya! Great job solving this challenge π
A couple of suggestions, but I'll try to keep this short:
ol, ul elements. Set their margin and padding to 0. After this, you can remove the left margin from your h6 elements. This should help line up the list elements with the h6 headings.a element. For the aside element, you can remove height: 25%, and set top and bottom padding instead. Once that's sorted, consider removing the started class from the button in your aside element.Hope this helped! Best of luck with future challenges!
Heya! Great work finishing this challenge. π
A few suggestions:
main.css) and your stylesheet with media queries (media-queries.css) into one stylesheet. That way you only make one request to the server.font-size: 0.6rem, the text ends up being about 9.6px. This can be a little difficult to read on smaller screens.Hope this helped a bit and best of luck with future challenges!
Difficulties faced:
Hiya! The finished product looks great by the way, so well done! π
I do have one suggestion for you though. Instead of using float and clear, I recommend you use flexbox instead. I think for the smaller parts of this project that you've used float on, it's fine. But as you start working on larger projects, you may want to learn more about things like flexbox and/or CSS Grid. Hope this was helpful!
