Manage Page - SCSS and JS (Siema for Carousel)

Solution retrospective
I had some issues with this one:
-
I thought from the design that the carousel was only needed on small screens. I have made this happen, but only on page load, as such if you change the size of the window the design won't respond. This works fine large to small screens, as the content is visible, but not if you start small and then make the window larger.
-
The carousel is problematic. It's not accessible, and the buttons for changing the image shown are too small. This is in line with the design, but I don't like it and my conclusion from this challenge is that carousels should be avoided at all costs - they seem rubbish from UI, accessibility and coding hassle perspectives.
-
For some reason I cannot get focus to move to the links inside my
main
element. It will move to thebuttons
(if they are there on small screens )but not the links. But outside themain
the same element with the same class works fine. I am not sure if it is the something in themain
causing the issue?
Any advice on this would be really appreciated. @grace-snow I wondered if you had a moment to look at this with your a11y eyes...
Any feedback and comments are most welcome. Thanks.
Please log in to post a comment
Log in with GitHubCommunity feedback
- @grace-snow
One more thing I thought of actually, triggered by @tediko's comment. Look into using the
current="page"
attribute and value in your navigation - @tediko
Hello, Dave! 👋
Congrats on finishing another challenge! Yet again, you did great job! Nothing to add after Grace exhaustive feedback, but hop in to say some kind words. Maybe small tip is that to make your alternative text/aria-labels more descriptive. For logo - "Manage - home page" and for social icons "Follow us on XXX" etc.
Good luck with that, have fun coding! 💪
- @grace-snow
Hi Dave,
This looks sweet, well done! 👏
The reason your anchor tags aren't focusing is they are missing the href attribute
Looking at your report for this, you need to change some other attributes too. If you want to add custom attributes you should be using
data-
to prefix those attributes (or are things like status and enabled part of the third party carousel maybe?)I agree, carousels are poor for UX and accessibility. Sadly, sometimes clients will insist on them. Heydon Pickering has written about them on Inclusive Components, so you might find that helpful: https://inclusive-components.design/a-content-slider/
I don't think you need any of the aria-labelledbys on your sections. I tested this on a project of mine recently and found it just added bloat of extra announcements and didn't help me navigate the page with my screenreader. Those sections all have headings anyway. If you thought they needed extra affordance and the content is able to standalone, you could make them into articles if you wanted, but that's not essential either.
On your footer, only what you have as
nav-inner
should be in a nav element. (At the moment you have links to external social media inside that footer nav). Maybe consider using column style properties and placing all those footer links into one ul instead of two as well.I think you need to add a flex-wrap somewhere in your footer too (
.footer > .container.split
). At the moment, as I shrink the screen down, at some point the footer content breaks out of its container.With the mobile menu, you are nearly there... You just need to add
aria-controls
to the burger-menu__trigger button and an ID on the burger-menu__panel so those are linked to each other. At the moment it says it's expanded, but doesn't say what it controls.really nice work on this, again. I hope the feedback helps
Join 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