Hey friends,
I’ve built a very simple random color generator, that changes both the background and the button to a random RGB color.
I’d like to know where I can improve my code (i.e. refactorings, design etc.).
Any suggestion will be welcome, I’m eager to learn as much as I can!
Cheers,
Andrea
Can we see the code?
Nice work!
I think one change I’d make is to only use Math.random once and do it in a function that only returns a number between 0 and 256. Then I’d use that function inside the function randomRGB specifically inside the string interpolation ${}. For example:
rgb(${randColorNum()}, ${randColorNum()}, ${randColorNum()})
I like this kind of separation of concerns:
One function returns a number between 0 and 256
One function returns a color with rgb()
Also this part inside the event handler:
body.style.backgroundColor = randomRGB();
btn.style.backgroundColor = body.style.backgroundColor;
If both elements will share the same color then I’d make a variable called color.
const color = randomRGB();
body.style.backgroundColor = color;
btn.style.backgroundColor = color;
Otherwise if you want the button color to be different than the body BG color, then remove the color variable and just call the randomRGB() as the values for both the button and BG color.
I noticed you set outline: none on the button. Is there a reason you did this? You should always have a clear keyboard focus indicator for any controls that can receive keyboard focus. This is an accessibility issue.
P.S. You don’t technically have to use outline to create the focus indicator. You could use the border instead. Or change the styling of the button in some way that is clear the keyboard focus is on the button.
Hi, thanks a lot for the detailed explanation! Exactly what I was hoping for.
That code definitely needed some order!
Hi bbsmooth,
You’re right, I haven’t really though about keyboard focus! What I wanted to achieve is basically to eliminate that button frame that stays there when you click on the button, and in order to make it disappear you have to click elsewhere. How can I achieve that, and yet still be able to have keyboard focus? Thanks
You mean you want to eliminate the border around the button? You could just set border: 0 on the button. I’m not sure I would do that though. Most people expect some sort of indication of where they are supposed to click so having the border outline on the button is probably a good thing to have.
Regardless, a keyboard user needs to know where the keyboard focus is on your page so you will need to show some sort of focus indicator on that button. There is a way that you can show the focus only for keyboard users (i.e. the indicator won’t show if you click on the button with your mouse or finger):
Yeah, that’s exactly what I was talking about! No need to eliminate the border altogether, I just wanted the focus effect only for keyboard users, not when one clicks. Thanks so much!
A lot of people I’ve coded with think the outline focus is kind of ugly even though it is helpful.
I’d say for personal projects, it’s not a big deal to remove it. But for projects where other people will use your app, then you can also consider styling the color of the outline to match the styling of your app. See here for some outline properties.
I would be very wary about messing with, let alone removing accessibility features like the focus outline. Those are there for a reason and there are people that depend on them. Even if it’s just your own project, I presume that you are going to showcase this at some point. If I saw that, it would make me think that this person lacks real world experience and does not understand accessibility issues.
It’s very common for new developers to hate things like the focus outline and want to get rid of it. Just know that it is part of the language of the internet and is expected to be there. The door knob disrupts the symmetry of my door. But I’m not going to remove it.
Thanks for the link, I’ll check it out!