@pikapikamart
Posted
Hey, really nice work on this one. Overall, the layout for both desktop and mobile state looks really great. The reply also works great as well the upvote and downvote.
For some suggestions, here are some:
- If you try to click the first upvote of the first comment, you can't point to it , I think it is because of the
h1being sr-hidden. You can addclip: rect(0 0 0 0);on theh1to fix it. - For each of the upvote and downvote, currently it doesn't have any informative text inside it and
+or-alone is not enough. What you can do is that, add ansr-onlytext inside it to which could bedownvote commentandupvote comment, depends on where it is placed. - Also, I would suggest changing the html placement for each of the post. The post's body should be placed before the ratings so that when a user navigates on the component, they will traverse the user, the user's post before the rating. Because it would be confusing to first traverse the upvotes/downvotes when the content is not first reviewed by the user.
- Adding an
aria-liveattribute on the element wrapping the counts so that assistive tech will announce the update to the user on what happened. You can add extra text if you want, ansr-onlytext. Also, you could change thespanto justptag. - For each of the person's
img, since their name is already present, it would great to use their name as thealtvalue since it makes sense to do so. - Actually, if this were a real app, each person would be a hyperlink right. Maybe making them wrapped inside
atag? Each person'simgand name. Could be right. - Whenever you wrap a text, always use meaningful elements and not
div. Changing all thosedivthat wraps a text should be replaced byptag. - For the reply
button, the arrow-svg should have anaria-hidden="true"attribute on it since it is only a decorative image. - Again for the reply
button, addingaria-expandedattribute would make it more accessible. This way, when a user toggles it, assistive tech will inform the user that it shows or expanding something. - For the reply
form, using the user's name on the user's avataraltattribute would be nice. - Adding
labelfor thetextareawould be great. Remember to always addlabelfor eachinputortextareaelement. This way, when a user is using a different language, those label text will be changed as well since attribute's data is static, theplaceholderwon't be translated. Thelabelwill besr-onlyon this one. - There is a confusing part on the
textarea. Initially, it should be empty right because a user is still not typing anything, but since you are already adding the@_nameon field, a user can press thereplyto send that reply even if they haven't add anything on thetextareaand it will be much confusing since it is usingrequiredand to be honest, I can't think of an approach on this one, so maybe just letting you know about this? :>> - When clicking now either the reply or cancel
buttonit would be nice again to have anaria-liveelement or maybe you can add a toast-notification on this one. The toast's text-content will vary on the user's choice, it could be likesuccessfully replied to {person's name}or could be likereply cancelled. Just remember to add anaria-live="polite"on the element so that it will be announced. - When toggling the delete
button, it would be nice to set the focus to the modal. This way, user will immediately be informed on what is the content. Right now, if you use keyboard to toggle the delete, then tab again, the user won't traversed on the modal like what it should do. So when a user toggles the delete, add thearia-expandedattribute then shift the focus in the modal, so that if they tab again, they will tab inside the modal. Another feature to add as well is to have a trap focus inside the modal. Meaning they can only tab on the two options, this way the user won't be confused on where they are at now. - To be honest, there are lots of added
aria-liveon this one or some toast, for the successfully deleting a reply and adding a comment as well.
Just in general, if there are lots of state in your app that requires to send an update to the user, an aria-live would be nice or sometimes changing the focus on the pop-up element is really nice.
Aside from those, great job again on this one.
Marked as helpful
@AchrefFast
Posted
@pikapikamart
Hi Raymart. Thank you so much for taking the time to provide me with such detailed feedback. I sincerely appreciate your help.
I tried to fix all the points that you mentioned in your reply, which most of them was new to me, and that really helped me a lot to look into new terms and features I didn't know about before, so thanks again.
For the Web Content Accessibility (WAI-ARIA), I couldn't find a good resources that can help me improve my understanding of these concepts well enough. Can you please give some advise where I can look.
Finally, I would be really happy if you can get another look at my solution (if you have time, of course).
Thank you again 😊😊😊🙏🙏🙏.
@pikapikamart
Posted
@AchrefFast Hey, glad that you find it useful^^
Actually, I don't have a specific resource about web accessibility, I just kind of search what I need. For example, on what I suggested about aria-live, you can go mdn or just search something like aria-live mdn or you can go into w3org. Both of these sites are really really great, especially w3org since it is the advocator or go to when checking about accessibility. Have a go on either on this one.
Also, sure! But maybe this lunch time on my end, morning coding should be first right :>> But looking forwards on the site again!
Marked as helpful
@AchrefFast
Posted
@pikapikamart Hi raymart, thank you for your suggestion. I will make sure to take a look at both sites.
Thanks 😃.
@pikapikamart
Posted
@AchrefFast Hey, I just reviewed again the site and it still looks great as before and great job on implementing those other suggestions I mentioned before!
Seeing again, I missed some little things:
- For each user's images, the word
avatarcould be removed since it is already an image. When usingaltyou don't need to add words that relates to graphic likeavatar, icon, image.... - Some text needs to be inside a meaningful element, currently, the
divstill contains some text and needs to be changed intoptag. - For each of the upvotes/downvotes
ptag, you don't need to addrole="region"on them, making them so will add extra navigation on assistive tech when traversing landmark elements. Removing those will be really nice. You only want to userole="region"to sections of the page where you want to highlight it. - Your
textareais missing theidattribute, you should change theforinto anidon that one. Yeah, the add commenttextareaand each of the replytextareaare usingforinstead ofid. - I see that you changed the reply
textareato not submit the data when the user hasn't type any value. Neat! Though currently, when submitting an empty form, the error is only seen visually by adding that redborderright. What you can do to improve is that, you can add anotheraria-liveelement that will announce if the form submission is invalid. For example:
if ( form is invalid ):
reply1Region.textContent = "Error. Please add text to the comment field";
reply1Textarea.setAttribute("aria-invalid", "true");
if ( form is valid ):
reply1Textarea.removeAttribute("aria-invalid");
In general, when using an aria-live element, in order for it work is that the element should already be present inside the dom. Changing the text-content of the aria-live element will make it be announced by assistive tech, but if the element is not already present in the dom, and you just showed it with it's text content, even if it has the aria-live attribute, assistive tech won't announced it.
So for the each of the toast-notification, the element that is using the aria-live should be present initially. Then you just changed the text-content of that, but do not remove it from the dom, just hide it visually^^
I think that's about it:>>
Marked as helpful
@AchrefFast
Posted
@pikapikamart Hey Raymart, thank you again for your precious suggestions .
Dealing with these different "ARIA" concepts that you mentioned helped me get a good idea about how important and crucial accessibility is in a website, and I think working on them while building the website is the right way to go, because trying to add them afterward is kind of frustrating and things can get too complicated.
I think I manged to fix all the points that you addressed, so thank you again for taking the time to look into my solution. I appreciate the help so much.
Many thanks 😊.
