Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Manage Page - SCSS and JS (Siema for Carousel)

P
Daveā€¢ 5,245

@dwhenson

Desktop design screenshot for the Manage landing page coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
3intermediate
View challenge

Design comparison


SolutionDesign

Solution retrospective


I had some issues with this one:

  1. 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.

  2. 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.

  3. For some reason I cannot get focus to move to the links inside my main element. It will move to the buttons (if they are there on small screens )but not the links. But outside the main the same element with the same class works fine. I am not sure if it is the something in the main 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.

Community feedback

P
Graceā€¢ 28,550

@grace-snow

Posted

One more thing I thought of actually, triggered by @tediko's comment. Look into using the current="page" attribute and value in your navigation

1

P
Daveā€¢ 5,245

@dwhenson

Posted

@grace-snow thanks! Added to the list of things to update! I actually used to include this one but seemed to have forgotten about it!

1
P
tedikoā€¢ 6,580

@tediko

Posted

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! šŸ’Ŗ

1

P
Graceā€¢ 28,550

@grace-snow

Posted

@tediko you don't need "follow us on..." - screenreader users don't like extra bloat. If an icon is enough for everyone else, the name of the destination is enough for assistive tech šŸ˜‰

1
P
Daveā€¢ 5,245

@dwhenson

Posted

@tediko thanks for the kind words! Always appreciated!! I had a goog poke around your IP tracker solution- really nice!

1
P
tedikoā€¢ 6,580

@tediko

Posted

@grace-snow Thank you for correcting me. I've read that more descriptive titles are good but i see your point. In the case of the logo, when the name of the page itself will not tell the user what will happen after interaction it is good to add some extra info on it but on the other hand in the case of socials links when Facebook/Twitter are self-defining there is no need to add more to it. Noted šŸ˜Š Fortunately, it's not harmful and the people I gave this advice won't be angry at me šŸ˜…

1
P
tedikoā€¢ 6,580

@tediko

Posted

@dwhenson Thank you! That means much to me :)

0
P
Graceā€¢ 28,550

@grace-snow

Posted

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

1

P
Daveā€¢ 5,245

@dwhenson

Posted

@grace-snow thanks so much! I'll go thought the points you raised - really appreciate all the advice.

Can't believe it was because I was missing the href! Note to self, check the HTML first next time!!!

One last follow up question if you have a moment: why should the external links to social media be outside the nav? Are they not considered part of the navigation?

Actually, forget it as I type that sentence, I can see they are not!

Thanks again.

0
P
Daveā€¢ 5,245

@dwhenson

Posted

@grace-snow Thanks again for the inputs. On the burger-menu, I'm basing this on a tutorial Andy Bell put out so I thought I'd ask him if he thought about adding aria-controls as the tutorial was really in-depth. He actually responded! And said:

"that's a legacy role now the relationship is implied with the existing relationship in the DOM of trigger and panel. If they are far apart, it wouldn't hurt to put aria-controls on there, I suppose. I haven't come across it for a long time, personally".

I don't have any basis for an opinion here, but I just thought you'd be interested in his response. I am finding that this a11y stuff seems to be much more nuanced and harder to get solid consensus on that other areas I'm learning.

1
P
Graceā€¢ 28,550

@grace-snow

Posted

@dwhenson yeah that's really interesting. I don't find my screenreader does anything different with them but figured it's just a setting I haven't turned on. I'll have to look more into this one I think. Thanks

1
P
Graceā€¢ 28,550

@grace-snow

Posted

@dwhenson whoaaah I ma not gonna bother with those attr any more I think šŸ˜‚ https://heydonworks.com/article/aria-controls-is-poop/

1
P
Daveā€¢ 5,245

@dwhenson

Posted

@grace-snow ah, great! Glad the info was useful!! Heydon's article is great thanks. I like his writing.

0

Please log in to post a comment

Log in with GitHub
Discord logo

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