Design comparison
Solution retrospective
Your feedback is much appreciated.
Community feedback
- @pikapikamartPosted about 3 years ago
Hey, awesome work on this one. Though I had to zoom out to see the desktop layout, are you using wide screen? I am using 1366x768 resolution and upon seeing the css, you are using 1440px as a breakpoint which is too big for all mobile layout to occupy. The 1440px on the design does not mean it is a breakpoint, it is just the size where the layout is created and not related to breakpoint. Also it is kind of confusing on why you did this since you started on mobile first approach which should not be the case. The mobile layout however looks great.
Some other suggestions would be:
- It would be great to have a base styling with these properties
html { box-sizing: border-box; font-size: 100%; } *, *::before, *::after { box-sizing: inherit; }
This way, handling an element's size will be lot easier because of the
box-sizing: border-box
. You can search up some more of how this works.- Avoid using
height: 100vh
on a large container like thebody
tag as this limits the element's height based on the remaining screen's height. Instead usemin-height: 100vh
, this takes full height but lets the element expand if needed. - Also don't add
width: 100vw
as this adds an extra horizontal scroll since this doesn't account the scrollbar's size. - The element usage on the small-blue-section-text is not correct. When wrapping a text-content do not just use
div
to wrap it, use meaningful element like ap
tag if it just a regular text or heading tag if it is an heading. - The error-icon
img
should be using analt
. Always remember to include thealt
attribute of theimg
element even if it is empty or not. If you don't screen-reader will read the source path of theimg
which you don't want. - Right now, the error-messages can only be seen by sighted users since it is not properly linked up on the
input
element. A pseudocode of a form or just aninput
in general when it is error should look like:
if ( input is wrong ) input.setAttribute("aria-invalid", "true"); input.setAttribute("aria-describedBy", id of the error-message); else input.removeAttribute("aria-invalid"); input.removeAttribute("aria-describedBy");
The error-message element should have an
id
attribute which will be or is referenced by thearia-describedBy
attribute on theinput
element. By doing this, you users will know what kind of error they had made because of thearia-describedBy
that points to the error-message's content and the users will know that theinput
is wrong because of thearia-invalid
.- To make it more informative, an addition if you like, you could make an
aria-live
element that announces if the form submitted is a success or if it has any error. This way users will be informed right away if something is good or bad. - If you are new to what I mentioned, have a look at this simple accessible form that I have see the usage of the different attributes and how they are used.
- Also I forgot, your
input
tag lacks an associatedlabel
tag on it. Since there are visible-label, thelabel
would be a screen-reader only label, meaning it would make user of likesr-only
class. The text-content should describe what theinput
needs like the value on theplaceholder
. On what I send, you can see usage of this as well. - Lastly, the
footer
should be placed on its own row outside of themain
since it is a primary landmark. A typical layout of a site looks like:
<header /> <main /> <footer />
This way, all element that has content are inside their respective landmark elements.
If you need help, just drop it here okay. Aside from those, great job again on this one.
Marked as helpful1 - @FatimahFarooqPosted about 3 years ago
I really appreciate your detailed feedback.I cannot thank you enough.What you've mentioned earlier regarding the 1440 px not being a breakpoint rather it's the layout size,I would like you to explain this a bit,if it's no trouble. Regarding what you mentioned about setting box-sizing:border-box,you are right,I need to find out more about this as I'm seeing it being used in all tutorials. Using proper html tags and adding an alt are such basics,I can't belive to have missed those..lol,will pay more attention.
l'll be mindful of the height and width property as well,what you pointed out was great. The error-message attribute (aria-*) is new to me, I'll definetely read more about it. I'll check out the link you've shared too.Thank you so much,your feedback really gave me a lot to check out and learn.Thanks again.0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord