@haquanq
Posted
Hello @justinsane 👋
This is my feedback on HTML markups:
- Page must have one
mainlandmark (read more here)) which means wrap the mot important content of the page inside it. - Consider using more semantic elements such as
section, article, figure,..to wrap a part of content. For example, instead of wrapping your card withdivyou can consider it as an article or section. - Avoid unnecessary
divwrapping, keep your HTML simple. Becausedivhas no semantic meaning (choose div when there is no other elements that fit the use case), you should leave the content as it is if the layout does not need special style control (like flexbox)
When ever you want to send data a way you should use form element with proper input type to represent what type of data users need to provide. In this use case, you can see there is 5 points (scores) from 1 to 5 that users can only choose one, you should use input type="radio" for each score button. Further more, you should wrap all radio buttons inside fieldset (read more here) with legend to describe a group of inputs.
About your react code, i could say these above information will change the way you handled submit event for now.
Have a nice day and happy coding! 😁😁😁
Marked as helpful
@justinsane
Posted
@haquanq Thanks for the great feedback.
-
incorporate semantic html
-
avoid unnecessary
divwrapping- still working on this one. I feel this is a byproduct of React?
-
Used the correct
formwithtype="radio"
See my Rating component below:
function Rating({ setRating, handleSubmit, rating }) {
const handleRatingChange = (e) => {
setRating(Number(e.target.value));
};
return (
<section>
<div className="relative mb-9 flex h-11 w-11 items-center justify-center">
<span className="absolute left-0 top-0 h-11 w-11 rounded-full bg-light-gray/10"></span>
<img src="./icon-star.svg" alt="star image" className="relative z-10" />
</div>
<h1 className="mb-3 text-3xl font-medium">How did we do?</h1>
<p className="mb-6 text-light-gray">
Please let us know how we did with your support request. All feedback is
appreciated to help us improve our offering!
</p>
<form onSubmit={handleSubmit}>
<div className="flex justify-between">
{[...Array(5).keys()].map((_, index) => {
const value = index + 1;
return (
<label key={index}>
<input
type="radio"
name="rating"
value={value}
className="hidden"
onChange={handleRatingChange}
/>
<div
className={`group relative mb-4 flex h-14 w-14 cursor-pointer items-center justify-center rounded-full ${
rating === value ? "bg-white" : "bg-light-gray/10"
} hover:bg-orange`}
>
<p
className={`relative z-10 font-bold ${
rating === value
? "text-very-dark-blue"
: "group-hover:text-very-dark-blue"
}`}
>
{value}
</p>
</div>
</label>
);
})}
</div>
<button
type="submit"
className="mb-3 mt-2 w-full rounded-3xl bg-orange p-3 text-center font-bold uppercase tracking-widest text-very-dark-blue hover:bg-white sm-375:mt-0"
>
Submit
</button>
</form>
</section>
);
}
@haquanq
Posted
@justinsane example about not using div:
- Replace the
divwrapper of inputs withfieldsetalong withlegendas such:
<fieldset>
<legend class"visibility-hidden">Please rate us... (tell users concisely what to do with these inputs)</legend>
... inputs render...
</fieldset>
- The
legendwill stay hidden (this help screen readers on providing more context for how are users supposed to do when they focus on the input for the first time)
