I have tried to follow all the best practices. If you have suggestions do let me know.
Feedbacks are always appreciated.
Can anyone tell why in the screenshot not everything is visible when everything is okay. Same thing has happened in my previous solution.
Thank You,
#happyCoding
Your solution looks good I like the animation, here are some notes to consider.
The images are stretching when scaling down to small screen sizes, using the CSS property background-size: cover; will help you in this scenario.
Some interactive elements such as the menu button need to be rapped inside a <button> tag, choose the right semantic HTML element.
Happy coding.
I liked the development process of this project, the development of this Landing Page was very satisfactory.
send me feedback
Hi there, your solution looks good, is nicely responsive and I love the animation on the header, here are some suggestions concerning accessibility.
aria-hidden=” true”<button> instead of a <div>.Good luck
This is a temporal solution for the JS of the accordeon
Hi there, since this is a temporal solution I am dropping some comments that may help you to improve your solution next time.
On mobile screens, the text is overlapping, I think setting an explicit height may cause this kind of problem, so let the container shrink according to the text size.
Anything that is clickable or has a hover effect means it’s an interactive element, the toggle arrow actually it’s a button, so you can wrap the <img> inside a <button aria-label=” toggle arrow”>.
Illustration images are for decoration purposes, there is no information to convey so it's better to hide them from the screen readers using aria-hidden=” true”.
<body> is not taking the total height when scaling down to mobile screens, I would recommend using min-height:100vh instead of heigh:100vh to avoid this problem.
Good luck
Thanks to F.E.M. for putting up the site and setting up these free challenges. Feed back is welcome on any parts, but in particular I'm looking for anything, that would be more likely done in a working environment, if this had been done to work with a team of developers what kind of change would have been made?
Hi there, congratulation on completing this challenge, taking a quick look at your solution I think there are a couple of things that need some improvement
<h5> inside a <div>, <ul> is more adequate for that.[<h1> ..... <h6>] are used to construct a table of contents for a document, so avoid using heading elements to resize text. Instead, use the CSS font-size property.Happy coding
My first difficulty was align the dice and circle in the middle of the card. Then I learned two different ways to align content. The first one using grid/flex and the another was using margin( left/right) and the position.
Hi there, congratulation for completing your first challenge here 🎉, keep going, you gonna learn a lot by completing different challenges 💪
<button> tag. Take the habit of using the right HTML semantic elements for the right purpose.Good luck.
Light and dark mode while trying to keeping the users choice was fun to figure out so that the app would keep the users selection by using local storage
Hi there, Good job on this challenge, I am dropping some comments that may help you to improve your solution even further.
class=” App” is not taking the full-screen height, so instead of writing height:100vh, min-height:100vh will do the job.<div>, use <button> with an aria-label=” toggle theme”, since there is no textual indication of the button purpose, we still need to provide alternative text for users who use assistive technology, (the same thing applied to the delete button and the checkbox)Happy coding
Hi there, Good job on this challenge, your solution is nicely responsive, semantically looks good, I loved that you have used the <blockquote> tag to wrap the advice.
aria-label to the dice button so it will be recognized for those who are using screen readers.Happy coding
Hi there, Good job on this challenge, your solution is nicely responsive, though there are some cards that have different sizes compared to the others on mobile screens.
Here are comments about semantics and accessibility
<h2> here, the title “where in the world” and “dark mode” the theme button, first of all, button text shouldn’t be a title, in this case, it is better to use <h1> where in the world</h1> since it is a single-page website.<div> for the theme button use a <button>.alt text to the flag images since it is an informative image.“fitter by region”.Happy coding
Hi there,
<button> rather than an <img>, try to choose the right HTML semantic elements.Happy coding.
Hi there, Good job on this challenge I would love to drop some comments that may help you to improve your coding skills even further
<div> to most of your elements, for instance instead of using a <div> for the dice button use a <button> because it’s an interactive element.<h1> every website need to have an <h1> title, in this case, it is a bit subtler, so you can choose <h1> Advice generator</h1> then make it sr-only (screen reader only ) since there is no <h1> in the design .Good luck
My solution to this challenge, I would appreciate your feedback how to improve my JS.
Hi there, your solution looks good I am dropping some suggestions that may help you to improve your code even further.
1- Concerning the images, since they are for decoration don't keep them visible for the screen reader(we don't need it to read their source path).
2-The advice text instead of putting inside a <div> it's better to put inside a <p> tag.
3-Add an aria-label=" generate advice", to make the button readable by the screen reader.
Happy coding :)
Any suggestions are welcomed 🙏.
Hi, thank you for your detailed review, I rarely get one.
I will make sure to take everything you have mentioned into consideration 😃.
