@pikapikamart
Posted
Hey, awesome work on this one. Layout in desktop looks fine, just needed for it to be centered vertically. The responsive state could be better since right now, at 960px going down to 684px, a large section is being hidden thus creating a horizontal scrollbar. The mobile layout looks fine but a small scrollbar is present as well.
Some other suggestions besides the logic for the tip:
- The
alt
for the website-logo should have usedalt="splittier"
, the image already has the text so better using it. Also, as always avoid words that relates to "graphic" such as "logo", always use meaningful words for meaningful images. - For the
form
you are using too muchgrid
, actually you don't really need that in there, since the layout is just one column. Those tips is just one container with aflex
. - For the bill
input
add amin=0
attribute so that whenever using arrow-key, the value won't go lower down the specified value which is 0. - The dollar icon on the
input
should have usedalt=""
andaria-hidden="true"
as an extra attribute. Decorative images should be hidden for users. - To be honest, I find it hard to use structure html on the tips section, but for me, I would use
input type="radio"
on the 5 items which is being wrapped byfieldset
along with ansr-only
legend
element. This way, users will know directly what does this set of items will do and will be easier for them to select one. Because using onlybutton
does not add extra accessibility features, unless you usearia-live
element. - The custom
input
needs to have alabel
associated with it, thelabel
will be havingsr-only
class, then the text should describe what does theinput
does. - Again
min=0
for the custominput
. - For the number of people
input
even if I put 0 or negative values, the error-message doesn't seem to appear, you might want to check that one out. - The
alt
for the icon on the numberinput
should bealt=""
andaria-hidden="true"
. - The
tip amount
and thetotal
could use some heading tags, since it is a section that shows the value. - The dollar sign again should be hidden, using the above method that I mentioned.
- You don't have to use
input type="reset"
on this one, since theinput
is not inside theform
if you are going to use this, you must include it inside theform
to work properly. But since it is not, you might want to just usebutton
element for this one. As an extra accessibility, anaria-live
element announcing that the form has reset would be really great.
Aside from those, great work. But look at the responsiveness that was mentioned above.
Marked as helpful