Thank you so much for making this. I'm going to share this video with my team because you concisely captured years worth of "learning the hard way" in a short focused video.
As a developer at the beginning of my journey, I really appreciate you reminding us that everyone makes mistakes. I always learn so much from your videos but I especially appreciate how open you are about your process.
I know, I know, pedantic JS time. That collapsed if statement for controlling the menu feels really wrong to me. I understand why you've done it like that that to a degree, but you could definitely get rid of the if(...); surrounds so that you just have 'isOpened ? closeMenu() : openMenu()'. To me, empty if blocks just make me feel...off, even if it does work perfectly fine. Despite that minor bit, though, fantastic video! Excellent coverage on button and nav menu accessibility. The final bit of pedantry for me would be that maybe you could swap out for ? Apparently might signify to accessibility tools that the s within contain interactable content, unlike the information content of a , but I don't think any tools really care at the moment and according to the MDN it looks like they're both considered virtually identical by the browser. Still, could be something that might become more relevant in the future. Or not.
first, love the videos! been watching for a year or two now! second, while this didn't happen in the video, if someone did that and forgot the semi-colon at the end of the line, the first line after that will technically be guarded by that if statement which could induce unanticipated behaviors since there were no scoping braces after that
I agree with @JBird that knowledge of the language is just a pre-requisite to reading code. That being said, I also agree that (IMO) the ternary operator isn't the most readable choice here. Personally, I use the ternary operator exclusively for expressions (think things that take on a value) while I use if exclusively for statements (think things that do something). Somehow, it feels more natural to have something like "x is either 1 or 2" or "I want to set the label to 'button' and then add either nothing or an s" than to do it with an if (plus it allows me to use something like const when I want to conditionally initialize a variable) and then for behavior I can say "if it's open, I want to close it, else I want to open it". Also, just as a quick side note on the expression vs. statement thing: in JS / TS, `if` is always a statement - it CANNOT take a value and is never an expression. The ternary operator is an expression and always takes a value; however, it can also act like a statement! Some languages handle this differently. In rust, for example, `if` also forms an expression and for that reason, rust simply doesn't have a ternary operator! Instead, you can write something like `let x = if condition { 1 } else { 2 };` instead of `const x = condition ? 1 : 2;`. Personally, I would argue that this supports my view of when to use the ternary operator and when to just stick with `if`; even if it's slightly more verbose.
@@lucidattf Well, to be honest it confused me for a moment, because I did not understand why there was an 'if'. So I had to reason about that only to reach the conclusion that there was no reason for it. People often have different ideas on what is clear code, but I don't think adding a statement that does nothing at all makes it any clearer. In addition, it does come with the risk that someone later on is going to think "that semicolon must be there by mistake, the next statement should only run if that condition is true. Because why would there even be an if-statement otherwise? Lemme remove that semicolon".
"It's our job to do it properly" is pretty much how I define my career of coming in to projects that are behind and broken and having to fix them. Thanks for all you do, it helps to know resources like you are out there to direct my developers to for some concise and well built coding examples.
With all due respect, screw those people giving you push back on such amazing coverage. We're lucky to have you uploading such wonderfully full and robust content. I think we'd all appreciate you pointing out how to make sites more accessible. I think we all appreciate how you teach the right way, not the fast way.
Really good to see people A) be willing to admit and address mistakes they make and B) take assistive technology seriously. I'm not good at assistive stuff, but I do care about it and a lot of it is pretty subtle imo, so it's really nice to see it being addressed like this.
Owning up to your "mistakes" and showing why and how to fix them is the definition of a true top level humble dev. You should never be afraid to own up, great work on this video !
I just graduated college and your videos actually helped me get a job. I’ve been taking everything you say and right notes, and apply it to websites I make for my portfolio. I talk about it in my interviews and got a job as a remote full stack developer who focuses on front end! Thank you so much Kevin! I’ll continue watching your content, to continue learning!
I am pretty sure you don't need the if condition for the ternary operator. so, instead of if(isOpened ? closeMenu() : openMenu()) It should just be isOpened ? closeMenu() : openMenu(). Also I think the closeMenu and openMenu should be the same function with an argument of the true/false Let me know what you think
Yup, you're right. You don't need the if(…) around the ternary operator. The closeMenu() function is redundant, instead you can use a ternary operator as second argument for the setAttribute(): siteNavigation.setAttribute('data-state', isOpened ? false : true). Last thing to mention: instead of adding a third state (closing) I would use the event listener 'transitioned'; after the closing animation/transition you set display to none. That saves you nearly 40-50% of code lines.
I don't think it should be the same function, firstOff - it's less readable and on top of that it will lead to over engineering like this: function toggleMenu(state) { menuToggle.setAttribute('aria-expanded', state); siteNavigation.setAttribute('data-state', state ? "closing" : "opened"); if (!state) { // we're closing siteNavigation.addEventListener('animationend', () => { siteNavigation.setAttribute('data-state', 'closed'); }, { once: true }); } } or you could use the *animationName* property see ( developer.mozilla.org/en-US/docs/Web/API/Element/animationend_event ) function toggleMenu(state) { menuToggle.setAttribute('aria-expanded', state); siteNavigation.setAttribute('data-state', state ? "closing" : "opened"); siteNavigation.addEventListener('animationend', () => { *if (e.animationName === "clipPathCircleClose")* siteNavigation.setAttribute('data-state', 'closed'); }, { once: true }); } Either way, it gets very complex, very quickly. It's just better to leave it as is.
Honestly this shouldn't even be a function it should just toString the opposite state of isOpened. ```js menuTogglesetAttribute('aria-expanded', !isOpened.toString()); ```
Yea you can use isOpened ? closeMenu() : openMenu; - I use these a lot when setting up variables conditionally and need a specific true or false value assigned.
I love that you didn't just delete the video and pretend like it didn't exist. Instead, you took the time to make a whole new video explaining why it was a mistake, and how to correct it. Love your channel!
When you added your poll, I knew it was this one!!! Who hasn’t done that at some point? My Senior Dev at my first job didn’t care about clickable divs, so I was off to a poor start. I eventually realized it was an issue, but I had a hard time properly evangelizing to the rest of the team. After switching jobs, we’re now extremely a11y aware, and it’s incredible. Really thrilled to see you bringing awareness here! Thank you.
Kudos to you for taking the time to actually make a video correcting mistakes and explaining it rather than just pinning a comment with the correction!
Passing a non-string second parameter to setAttribute() will convert it to a string anyway. So the code around 13:05 can be simplified to menuToggle.setAttribute('aria-expanded', !isOpened); No need for a ternary operator or extra functions for that. It's also worth knowing that there's an optional second parameter to classList.toggle() which can be used to force the addition or removal of the class - making it behave like classList.add() or classList.remove() depending on the value of that second parameter. That would allow you to do something like this: menuToggle.classList.toggle('open', !isOpen); ...without having to worry about things getting out of sync if the menu is already opened when the user clicks. Finally, you should always check the 'animationName' property of the animationEnd event (at 29:34) to make sure your code is only responding to the menu animation, and not any others that might be added to the page. In this case I suspect you could have got away with just using a CSS transition with a transition-delay value on the 'display' property though, without needing to use animations at all.
Kevin, this video's title might be very incorrect! When I saw a 30 minute video and the title, I thought I'd watch / skim about 5 minutes of it to find the mistake, but ended up watching the entire thing. It has a ton of value! Thanks!
you Sir teach me that speaking a language doen't mean you actualy know it . watching videos here and there and watching your videos always makes me realise this truth
I also really appreciate to see you admitting to making "mistakes". Although I wouldn't refer to this as a mistake but simply "not the best choice made".
Thanks for showing different options and the accessibility coding. I really like that you have the split screen showing the code then what it does in real time. Very helpful.
Great video! I completely agree with your rant about doing it right being important. Also, one little nit... You don't need a surrounding if statement for your ternary (17:49). You can just use the ternary by itself. But, sometimes lint rules will complain even though it's completely valid. Adding the additional if statement makes it less readable to me since that if statement isn't actually used for anything.
I'm happy when you show that there's more work to this. Take away the marketing reasons and other reasons behind accessibility and the number one reason we should be making our sites accessible is that it includes people. No one likes being left out. So if I have to put a little extra work into a build, or go back and change some things in a build I made, just so a few more people can feel included if they stumble across my site, then that alone should make it worth it. Half the reason I watch your videos is for stuff like this.
I've seen you use these data and aria attribute selectors in your CSS before, and I feel like it's finally clicked for me as to why this is such a good idea. If I am reading the CSS, seeing that a style is applied when aria-expanded = opened, makes the intent and meaning of that style a lot clearer. Another side benefit of doing things properly.
As someone who primarily works on internal tools that don't face the public I really don't often think of developing for people who need to use assistance tools to get around the web, like someone who is blind. I fully came into this video thinking "who cares about using a div, its not that big of a deal.. you can make it all work the same" and within about 5 minutes realized I was forgetting that there are millions of people who need us to do more for them so they can use our products. Thanks for the great reminder that I need to consider that some people need us to put in a good effort instead of doing things the lazy way.
The only accessibility thing missing from this is what happens if the user doesn't have JS enabled :P. Always have a no-JS fallback as part of accessibility, too. Also, animations are super cool but they need to be able to be disabled from knowing the user's animation preference (prefers no animation). Obviously that is outside the scope of this video's concept, but they are things to keep in mind.
A no-JS fallback seems like an impossibility to me. There are far too many things that simply do not work without JS and there is no good way to add alternatives seamlessly. You'd just end up with a broken website.
CSS-only drop down menus have been around for many years, no JS required. Most examples on the web open on :hover, but we can use :focus instead so that it opens on click instead, and also when navigating with the keyboard.
Love your work.. I consider myself to be pretty OK with JS and CSS. However, I try to watch at least one of your videos each day. You have great clarity and I always find something interesting.
😅😅 You have no idea how many sites I have made using divs and spans as button, then add a javascript and cursor pointer. I have learned something new and important. Thanks Kevin!!
Thank you so much for this. Any job I work, no one cares about semantic HTML or accessibility or basic keyboard user experience. Awesome stuff, please keep doing this!
oh thank you man. I was doing the wrong way by simply adding too many setTimeout and intermediate classes (like closing or opening). My handheld menu bar is a real mess. You are great. Thank you again.
This is a great video! Thanks, Kevin! Not only does it say "Hey, we all make mistakes", but it shows us a great way of achieving a really solid, neat way of approaching accessibility in our designs. And as you say, this may be a lot of work now, but once it has been set up it's there for later. If web technologies improve again, then this can be expanded upon to include those new techniques.
I think this is by far my favorite video of yours in recent memory. I would like to see this navigation taken a step further to address the issue of responsive design. My biggest question I was left with was "What happens to the button element when you're on a desktop and how to you address the two different states of the menu in one solution?"
Unlike display the visibility property is animatable from hidden to visible so the clip-path transition should've worked with that. Although hidden from screen readers it remains in the DOM and has layout.
Thanks, Kevin! Even this turned out to become a more complex video, it's show your progress and how you achieved certain things you would never do the same way. Accessibility today is very important, so thanks a ton for this update and the good work as always.
I always type something along the lines of elem.setAttribute("aria-expanded", isOpen) And when toggling attributes I do elem.toggleAttribute("aria-expanded", isOpen). This will remove aria-expanded if it's not open, but add it if it is.
excellent video, i am the worst for taking the easy road when it comes to coding, but since watching your videos and taking a couple courses from you i spend more time analysing what i want to do and building my functionality one step at a time, and i do a lot more testing than i used to, your commitment to perfection is admirable, i am definitely going to incorporate your hamburger methodology in my future products, using the aria with the js code to eliminate css code is very smart, thanks Kevin
Another top notch video from the King. Thank you! Here's my contribution to it: instead of using to list menu items, use , which is the new semantic list wrapper for this purpose.
Instead of display none you can use opacity and pointer event none and keep using the transition for better performance; Also you can use aria-hidden in the navigation when hidden. BTW great video!
Nit: Let's profile that before jumping into optimization. Sadly the proposed solution for faking the `display: none` does not account for the keyboard navigation.
As others have said, you don't need the if with the ternary inside. You also don't need to create functions to use the ternary, you could have kept the same two lines as the if / else.
The issue with the event listener for "animation closed" is that you are creating them everytime you click the button. Even with the once is still there. Other option is to create the listener outside the function so it will only create once. Also if you pass the event as a parameter you might get context on the trigger so the same listener can be used for multiple closing signals
I think CSS property for .site-nav{visibility: hidden} perfect in this case. Because it supports CSS transitions and hides the ability to work with the keyboard. And also frees from lines in script.js I may have missed something, if so, please correct me. P.S.: I always admire open self-criticism on RUclips. Kevin, you are an amazing person and developer!
Elements set to {visibility: hidden} are removed from the accessibility tree, just like elements set to {display: none}. It would likely cause issues for screen reader users.
@@nilaallj our task is to hide navigation elements from the visibility tree when we need to hide this element. The display:none property didn't work due to missing animations. Why not use js instead visibility:hidden?
Was about to comment just that as well: Simply transition visibility one-way with a delay of the animation length or using steps(1) easing. In some scenarios you might also want to toggle position to absolute, if the element being animated is in the layout flow and would leave an unintended hole.
This is an awesome video! Very informative, and the display:block solutions is a good one. I normally would use aria-hidden or remove all the tab-index on the links ;) This solution is much cleaner IMO thanks for sharing!
Kevin! I truly admire how you are passionate about CSS. Everytime I see your videos I learn something new. I wanted to ask, what are some projects that you have worked like websites that are out live today in the real world. You are ledengary.
I've reviewed hundreds of job applications over the years, and the times I see front-end developers not having the faintest idea what W3C standards are is astounding. Sometimes I see them nesting and together, and similar nastiness. So many people simply don't understand that conflicting interactive elements shouldn't be nested. Many of them don't know there are rules to HTML, and many of them underestimate HTML. I mean, kudos to them for not ONLY using , but when you use semantics... use them correctly.
This is probably something that needs to be taught more, tbh. It's now 20+ years since I began to learn HTML and I was lucky enough to learn it from notes and literature that happened to talk at least a bit about a11y, but I get the sense most people that learn HTML in, say, university courses or whatever (1) just don't care that much about HTML in the first place and (2) don't really seem to ever get told that a11y is something you can improve in concrete ways if you write your code right. The 'move fast and break things' culture that I see in a lot of startuppy Facebook-wannabe circles doesn't help either, of course.
@@narnigrin (1) - It is too easy to blame university courses or whatever as those have very limited time to actually teach those things. (2) - Telling someone that writing code right will improve a11y won't magically make theme writing the code right ;) I personally think that at the end of the day nothing really replace work experience and as long as people are willing to learn and peers are willing to share their experience, HTML semantics shouldn't be a deal breaker
I commented this, but comment has gone... In this video you introduced a state-machine bug at around 13:30 and then do it more severe at 17:72 . Problem is not big enough in this particular case, but it’s very important when you introduce states and not carefully consider (and it’s easy to learn bad habits than fix these bad habits later on). Later you introduced one more state and didn’t verify or check that previous state machine is actually correct. At the beginning you had transitioning from „open” to „close” (and boolean type is a good enough to hold it), then you introduced „closing” state, but left check as boolean. When doing ANY optimisation, it’s important to repeat loudly: „Preemptive optimisation is evil” and „premature optimisation is evil”. If you do optimisation, do automated (preferably) tests before AND after. It’s very easy to reproduce if you click on menu button fast enough to close and open menu. The transitioning will be visually unpleasant and with a bit more complex state machine it’ll end up in an invalid state. The correct transitioning will be check non-boolean state and do following in on-click event function: * open -> closing * closing -> closing (state changes from closing to close by an event) * close -> open For sake of completeness and to avoid visual flashes (when clicking fast enough), I’d add opening state. What if a clicker scenario like this: „click on menu button, press tab (to select first item), press space (to activate first item), press shift-tab (to select menu button) and press space (to activate menu button)”. I assume here that selected menu item changes content on screen without moving to another page. To avoid such problems in the future, I recommend to create a function `menu_state(current_state)` which do all *state* transitioning and returning new state. On-click listener (in this case) will pass current state and get new one, if state has been changed, only then do a thing. This way a programmer can test state machine and communication with on-click separately. PS: I wasn’t a guy who did a comment you mentioned, but I agree with it PPS: It's better not to permanently delete a video, but add a link for correction to both the description and the pinned comment
JavaScript Improvements 2: A one-liner to toggle the aria-expanded property (two lines including the isOpen variable declaration) menuToggle.setAttribute(isOpen ? "false" : "true"); No need for extra functions, just one line :D
My preference when it comes to programming is to go against the convention as a means of security through obscurity, on top of actual security standards. So using the information in the beginning of this video I’ve implemented a custom event that can be added to div buttons and instead of adding it with addEventListener you add it with prepareEventListener and based on which event you pass it, in this case it’s “sitename.interaction” it will set a tab index that’s based on its depth in the dom from body and use it’s innertext node to give it a aria-name property if it doesn’t already have one. And if you pass in a controlled child as an optional parameter it will ensure through setting a custom property if the child hasn’t been prepared or remanipulating it’s tabindex and it’s childrens to ensure they are above the tab index of the parent button, so you can layout your html however you need for zindex purposes and still get desired accessibility features without using the standard button which can be scraped for by general purpose bad actor scripts. Oh and I named the event interaction because it listens for both mouse down and pointer down and whichever event fires first serves the callback assigned and the second event to fire recognizes it’s already being handled which allows for dev tools to switch between emulating a pointer device and normal site without refreshing the page to reinitialize the event listeners based on the user agent or screen size as a fallback
Assuming not or you prob would have used. I’m also confused why it’s not setting that data state of closed ie display none when the animation to open it runs. Is it just the order set up? So you click the button to open, animation runs and sets aria expanded to true , and data state to opened. Then you click again and sets to false, and closing, that animation runs and that’s what is targeted to listen for the ending. Guess that makes sense just kinda had to talk myself through it. That’s why it’s helpful to have in separate functions
Accessibility is still a mystery to me, pls could we have a vid on that. Things like the aria, keyboard accessibility, role, building accessible dropdowns, select, popups, modals etc. Would be so grateful 🙏
Good things should soon come to you for doing this. Keep it up Kevin! You should do a summary of interesting not obvious behaviors associated with pure HTML tags and attributes. I bet there is a lot there that developers should be aware of...
Love your videos Kevin, your newsletter and especially appreciate you revisiting old mistakes to improve accessibility! However, the JS part contains a mistake so I am writing this comment hoping to clear up future viewers questions. The isOpened being a string check is not a problem, most programming languages use enums or strings as checks for conditions. Making it a boolean by moving the check in the assignment is a good idea, because the code is more readable and it prevents the confusion with an if(isOpened===true) situation. The if around the ternary operator is a mistake, because the ternary operator evaluations work fine without it. if(test?doA():doB()); // mistake, the if does nothing test?doA():doB(); // basically a shorthand if else And regarding people complaining that your JS is not concise enough, and could be shorter, I disagree with their point. You make great educational videos, and your target audience is not a FE dev in typescript/React, writing ternaries and closures for minimum code size/maximum efficiency. There are even transpilers, compressors and browser optimizations which can figure out your if/else can be turned into a ternary. I can do concise and as a bonus create a reusable function for toggling: const toggleAttribute = (obj, att) => ((obj, att, valA, valB) => obj[att] = (obj[att] === valA ? valB : valA))(obj, att, 'true', 'false'); toggleAttribute (menuToggle, 'aria-expanded'); But I would not write that one-liner both for my future self and future people working on the project. In regards to fixing the mistake, I think the rest of the video is of too great value and a pinned comment just explaining that the ternary can stand on it's own is enough. Don't stress yourself out, over a line of code. Thank you Kevin for your videos and your newsletter, you're down to earth explanations brighten my mornings! 😃
25:00 Why don't you just transition `visibility` along with `clip-path`? It functions the same as `display: none` (at least when you position the element absolutely) and doesn't require the `@keyframes` trick or the animation event listener, nor the toggling of classes.
I tend to avoid visibility since it takes up space still, but yes, since it's absolutely positioned it could have been a little bit easier, and still removes it from the accessibility tree.
Just something to note at 17:50, you don't need to wrap the ternary operator in another if statement. Instead of: if(isOpen ? closeMenu() : openMenu()) Just write: isOpen ? closeMenu() : openMenu() The ternary operator is just a shorthand way of writing an if-else statement. A ? B : C A's truthiness is checked. B is executed if A is truthy. C is executed if A is falsy. If you have no "else" condition then you can just do "A && B", where A's truthiness is checked and B is executed if A is truthy.
I remember doing that… add “cursor: pointer”, add “tabindex” to make it focusable, etc. No need to do much of that these days w/ first class support. Not to mention the impact on screen readers and other assistive tech with the new tags, it really cuts down on effort all around too.
As a relatively new frontend dev, I've encountered literally _all_ of those things and dealt with them in the _exact_ same way you did. Thanks for inspiring some confidence!
Thanks for the video and idea! 👍One question. Why create data-state="closing" when you could move the `display: none` property inside clipPathCircleClose for a value of 100% ?
I find what you can do with CSS fascinating, not just you, but like CSS peeps in general can do wondrous things. As a javascript guy though it hurt a little bit to see you put the ternary operator inside an "if()", heh. The "?" and ":" are the operators so they don't need to be wrapped up inside another block (especially an if).
As a blind developer, I just want to say I love your content and love your focus on accessibility ;__;
How do you know what ur writing?
@@xijnin 🎙
I'd love to see more vids on accessibility - it's definitely the only area where I'm not very well versed in.
I concur! Color impairment solutions would be nice as well
Check out work from Sara Soueidan -she is a great a11y teacher
@@a1white Did your company wait until before or after a predatory lawsuit was filed? LOL.
I agree
same :D
Thank you so much for making this. I'm going to share this video with my team because you concisely captured years worth of "learning the hard way" in a short focused video.
Thanks for supporting kevin he's so great
paypig
As a developer at the beginning of my journey, I really appreciate you reminding us that everyone makes mistakes. I always learn so much from your videos but I especially appreciate how open you are about your process.
Stop brown-mouthing Kevin 😶
I know, I know, pedantic JS time. That collapsed if statement for controlling the menu feels really wrong to me. I understand why you've done it like that that to a degree, but you could definitely get rid of the if(...); surrounds so that you just have 'isOpened ? closeMenu() : openMenu()'. To me, empty if blocks just make me feel...off, even if it does work perfectly fine.
Despite that minor bit, though, fantastic video! Excellent coverage on button and nav menu accessibility. The final bit of pedantry for me would be that maybe you could swap out for ? Apparently might signify to accessibility tools that the s within contain interactable content, unlike the information content of a , but I don't think any tools really care at the moment and according to the MDN it looks like they're both considered virtually identical by the browser. Still, could be something that might become more relevant in the future. Or not.
first, love the videos! been watching for a year or two now!
second, while this didn't happen in the video, if someone did that and forgot the semi-colon at the end of the line, the first line after that will technically be guarded by that if statement which could induce unanticipated behaviors since there were no scoping braces after that
@@lucidattf Writing with readability in mind is great, but writing so people who don't know the language can read it, seems a bit too much.
I agree with @JBird that knowledge of the language is just a pre-requisite to reading code. That being said, I also agree that (IMO) the ternary operator isn't the most readable choice here. Personally, I use the ternary operator exclusively for expressions (think things that take on a value) while I use if exclusively for statements (think things that do something). Somehow, it feels more natural to have something like "x is either 1 or 2" or "I want to set the label to 'button' and then add either nothing or an s" than to do it with an if (plus it allows me to use something like const when I want to conditionally initialize a variable) and then for behavior I can say "if it's open, I want to close it, else I want to open it".
Also, just as a quick side note on the expression vs. statement thing: in JS / TS, `if` is always a statement - it CANNOT take a value and is never an expression. The ternary operator is an expression and always takes a value; however, it can also act like a statement! Some languages handle this differently. In rust, for example, `if` also forms an expression and for that reason, rust simply doesn't have a ternary operator! Instead, you can write something like `let x = if condition { 1 } else { 2 };` instead of `const x = condition ? 1 : 2;`. Personally, I would argue that this supports my view of when to use the ternary operator and when to just stick with `if`; even if it's slightly more verbose.
@@jbird4478 Ternary operations are not as well known, adding the word "if" doesn't cause any more issues and makes it more clear
@@lucidattf Well, to be honest it confused me for a moment, because I did not understand why there was an 'if'. So I had to reason about that only to reach the conclusion that there was no reason for it. People often have different ideas on what is clear code, but I don't think adding a statement that does nothing at all makes it any clearer. In addition, it does come with the risk that someone later on is going to think "that semicolon must be there by mistake, the next statement should only run if that condition is true. Because why would there even be an if-statement otherwise? Lemme remove that semicolon".
"It's our job to do it properly" is pretty much how I define my career of coming in to projects that are behind and broken and having to fix them. Thanks for all you do, it helps to know resources like you are out there to direct my developers to for some concise and well built coding examples.
With all due respect, screw those people giving you push back on such amazing coverage. We're lucky to have you uploading such wonderfully full and robust content. I think we'd all appreciate you pointing out how to make sites more accessible. I think we all appreciate how you teach the right way, not the fast way.
Really good to see people A) be willing to admit and address mistakes they make and B) take assistive technology seriously. I'm not good at assistive stuff, but I do care about it and a lot of it is pretty subtle imo, so it's really nice to see it being addressed like this.
Owning up to your "mistakes" and showing why and how to fix them is the definition of a true top level humble dev. You should never be afraid to own up, great work on this video !
I just graduated college and your videos actually helped me get a job. I’ve been taking everything you say and right notes, and apply it to websites I make for my portfolio. I talk about it in my interviews and got a job as a remote full stack developer who focuses on front end! Thank you so much Kevin! I’ll continue watching your content, to continue learning!
I am pretty sure you don't need the if condition for the ternary operator.
so, instead of if(isOpened ? closeMenu() : openMenu())
It should just be isOpened ? closeMenu() : openMenu().
Also I think the closeMenu and openMenu should be the same function with an argument of the true/false
Let me know what you think
Yup, you're right. You don't need the if(…) around the ternary operator. The closeMenu() function is redundant, instead you can use a ternary operator as second argument for the setAttribute(): siteNavigation.setAttribute('data-state', isOpened ? false : true). Last thing to mention: instead of adding a third state (closing) I would use the event listener 'transitioned'; after the closing animation/transition you set display to none. That saves you nearly 40-50% of code lines.
I don't think it should be the same function, firstOff - it's less readable and on top of that it will lead to over engineering like this:
function toggleMenu(state) {
menuToggle.setAttribute('aria-expanded', state);
siteNavigation.setAttribute('data-state', state ? "closing" : "opened");
if (!state) { // we're closing
siteNavigation.addEventListener('animationend', () => {
siteNavigation.setAttribute('data-state', 'closed');
}, { once: true });
}
}
or you could use the *animationName* property see ( developer.mozilla.org/en-US/docs/Web/API/Element/animationend_event )
function toggleMenu(state) {
menuToggle.setAttribute('aria-expanded', state);
siteNavigation.setAttribute('data-state', state ? "closing" : "opened");
siteNavigation.addEventListener('animationend', () => {
*if (e.animationName === "clipPathCircleClose")*
siteNavigation.setAttribute('data-state', 'closed');
}, { once: true });
}
Either way, it gets very complex, very quickly. It's just better to leave it as is.
Honestly this shouldn't even be a function it should just toString the opposite state of isOpened.
```js
menuTogglesetAttribute('aria-expanded', !isOpened.toString());
```
@@fortuneswake (!isOpened).toString()
@@VeaceslavBARBARII true, the order of operations would require it to be in parenthesis
Mini-course about accessibility would be amazing!
Thank you for your great tips Kevin! BTW you don't need the "if()" block, 'cause you already used the "?" operator.
Yea you can use isOpened ? closeMenu() : openMenu; - I use these a lot when setting up variables conditionally and need a specific true or false value assigned.
I love that you didn't just delete the video and pretend like it didn't exist. Instead, you took the time to make a whole new video explaining why it was a mistake, and how to correct it. Love your channel!
When you added your poll, I knew it was this one!!! Who hasn’t done that at some point?
My Senior Dev at my first job didn’t care about clickable divs, so I was off to a poor start. I eventually realized it was an issue, but I had a hard time properly evangelizing to the rest of the team.
After switching jobs, we’re now extremely a11y aware, and it’s incredible. Really thrilled to see you bringing awareness here! Thank you.
I learned a LOT... Learning from mistakes and fixing them is probably more helpful than perfectly clean tutorials.
I loved looking at the word 'butt' for 5 seconds there (2:46). Everyone makes mistakes sometimes, you just have to learn from them :)
Kudos to you for taking the time to actually make a video correcting mistakes and explaining it rather than just pinning a comment with the correction!
Passing a non-string second parameter to setAttribute() will convert it to a string anyway. So the code around 13:05 can be simplified to
menuToggle.setAttribute('aria-expanded', !isOpened);
No need for a ternary operator or extra functions for that. It's also worth knowing that there's an optional second parameter to classList.toggle() which can be used to force the addition or removal of the class - making it behave like classList.add() or classList.remove() depending on the value of that second parameter. That would allow you to do something like this:
menuToggle.classList.toggle('open', !isOpen);
...without having to worry about things getting out of sync if the menu is already opened when the user clicks.
Finally, you should always check the 'animationName' property of the animationEnd event (at 29:34) to make sure your code is only responding to the menu animation, and not any others that might be added to the page. In this case I suspect you could have got away with just using a CSS transition with a transition-delay value on the 'display' property though, without needing to use animations at all.
Kevin, this video's title might be very incorrect! When I saw a 30 minute video and the title, I thought I'd watch / skim about 5 minutes of it to find the mistake, but ended up watching the entire thing. It has a ton of value! Thanks!
Why do you always use a -tag inside a , when there is a -tag? Would that subject be worth a video?
you Sir teach me that speaking a language doen't mean you actualy know it . watching videos here and there and watching your videos always makes me realise this truth
I also really appreciate to see you admitting to making "mistakes". Although I wouldn't refer to this as a mistake but simply "not the best choice made".
Thanks for showing different options and the accessibility coding. I really like that you have the split screen showing the code then what it does in real time. Very helpful.
Glad it was helpful!
Great video! I completely agree with your rant about doing it right being important. Also, one little nit... You don't need a surrounding if statement for your ternary (17:49). You can just use the ternary by itself. But, sometimes lint rules will complain even though it's completely valid. Adding the additional if statement makes it less readable to me since that if statement isn't actually used for anything.
Why should you be ashamed? You are learning too. The shame would be if you never really cared. Good work Kevin!
I'm happy when you show that there's more work to this. Take away the marketing reasons and other reasons behind accessibility and the number one reason we should be making our sites accessible is that it includes people. No one likes being left out. So if I have to put a little extra work into a build, or go back and change some things in a build I made, just so a few more people can feel included if they stumble across my site, then that alone should make it worth it.
Half the reason I watch your videos is for stuff like this.
I've seen you use these data and aria attribute selectors in your CSS before, and I feel like it's finally clicked for me as to why this is such a good idea. If I am reading the CSS, seeing that a style is applied when aria-expanded = opened, makes the intent and meaning of that style a lot clearer. Another side benefit of doing things properly.
Amazing video! It helps knowing that even the best make mistakes and are constantly learning. I am not alone in this never ending but passionate walk
Great stuff, I love when you talk about accessibility, cause it's one area that I really struggle at! Keep them coming!
I haven't watched the video yet. I just wanted to say Kevin, you're an amazing resource creating great content. OK now I watch it.
As someone who primarily works on internal tools that don't face the public I really don't often think of developing for people who need to use assistance tools to get around the web, like someone who is blind. I fully came into this video thinking "who cares about using a div, its not that big of a deal.. you can make it all work the same" and within about 5 minutes realized I was forgetting that there are millions of people who need us to do more for them so they can use our products. Thanks for the great reminder that I need to consider that some people need us to put in a good effort instead of doing things the lazy way.
The only accessibility thing missing from this is what happens if the user doesn't have JS enabled :P. Always have a no-JS fallback as part of accessibility, too. Also, animations are super cool but they need to be able to be disabled from knowing the user's animation preference (prefers no animation). Obviously that is outside the scope of this video's concept, but they are things to keep in mind.
A no-JS fallback seems like an impossibility to me. There are far too many things that simply do not work without JS and there is no good way to add alternatives seamlessly. You'd just end up with a broken website.
Let’s make sure we include support for apple watches too!
CSS-only drop down menus have been around for many years, no JS required. Most examples on the web open on :hover, but we can use :focus instead so that it opens on click instead, and also when navigating with the keyboard.
I love that you answer the question "why" ❤️
Bravo! This is my favorite RUclips video this year. Perhaps of all time. Thank you.
Love your work.. I consider myself to be pretty OK with JS and CSS. However, I try to watch at least one of your videos each day. You have great clarity and I always find something interesting.
Kudos to you Kevin! The best mistakes are the ones we learn from! And as always, your in-depth explanation of why is very much appreciated! :)
Kevin this is great! Going back to something you didn't do right and correcting it openly👍
😅😅 You have no idea how many sites I have made using divs and spans as button, then add a javascript and cursor pointer. I have learned something new and important. Thanks Kevin!!
anyone can easy make any design using CSS but these small things is what makes us different from others. Thank you kevin for this awesome channel
I'm glad I followed a good developer who is inclusive :)
Thank you so much for this. Any job I work, no one cares about semantic HTML or accessibility or basic keyboard user experience. Awesome stuff, please keep doing this!
oh thank you man. I was doing the wrong way by simply adding too many setTimeout and intermediate classes (like closing or opening). My handheld menu bar is a real mess.
You are great. Thank you again.
This is a great video! Thanks, Kevin!
Not only does it say "Hey, we all make mistakes", but it shows us a great way of achieving a really solid, neat way of approaching accessibility in our designs.
And as you say, this may be a lot of work now, but once it has been set up it's there for later. If web technologies improve again, then this can be expanded upon to include those new techniques.
I think this is by far my favorite video of yours in recent memory. I would like to see this navigation taken a step further to address the issue of responsive design. My biggest question I was left with was "What happens to the button element when you're on a desktop and how to you address the two different states of the menu in one solution?"
Vim user here and a frontend dev.
Thanks for making this video. I was confused when vimium didn't recognised a toggle button which was actually a div.
Great video.
Another option to exclude navigation from a11y when it is hidden is to add a new inert attribute.
Unlike display the visibility property is animatable from hidden to visible so the clip-path transition should've worked with that.
Although hidden from screen readers it remains in the DOM and has layout.
I wonder if he could have added "visibility" straight into the ClipPathCirlcleClose, showed at 25:50 instead of all those sketchy shenanigans.
Thanks, Kevin! Even this turned out to become a more complex video, it's show your progress and how you achieved certain things you would never do the same way. Accessibility today is very important, so thanks a ton for this update and the good work as always.
I always type something along the lines of elem.setAttribute("aria-expanded", isOpen)
And when toggling attributes I do elem.toggleAttribute("aria-expanded", isOpen). This will remove aria-expanded if it's not open, but add it if it is.
excellent video, i am the worst for taking the easy road when it comes to coding, but since watching your videos and taking a couple courses from you i spend more time analysing what i want to do and building my functionality one step at a time, and i do a lot more testing than i used to, your commitment to perfection is admirable, i am definitely going to incorporate your hamburger methodology in my future products, using the aria with the js code to eliminate css code is very smart, thanks Kevin
Happy Holidays Kevin, great video, about my fifth time watching it, always learn something new.
Another top notch video from the King. Thank you! Here's my contribution to it: instead of using to list menu items, use , which is the new semantic list wrapper for this purpose.
Great video as always. Just one note: if you use ternary operator, you don't need to wrap it into another if statement.
yup, went slightly too fast there when refactoring, lol. Luckily it's just redundant, rather than something that will cause a problem :D
Instead of display none you can use opacity and pointer event none and keep using the transition for better performance; Also you can use aria-hidden in the navigation when hidden. BTW great video!
Nit: Let's profile that before jumping into optimization.
Sadly the proposed solution for faking the `display: none` does not account for the keyboard navigation.
Also, really appreciate you taking the time to talk about why this is important and that we should continue to try to do better 💪
Wow! Accessible web design! Something that is overlooked by pretty much all “RUclips web designers”. Thanks for this video! 👍
As others have said, you don't need the if with the ternary inside. You also don't need to create functions to use the ternary, you could have kept the same two lines as the if / else.
Wow, this is one of the most informative videos I have ever watched
The issue with the event listener for "animation closed" is that you are creating them everytime you click the button. Even with the once is still there. Other option is to create the listener outside the function so it will only create once. Also if you pass the event as a parameter you might get context on the trigger so the same listener can be used for multiple closing signals
This is the best tutorial video on the internet
16:20 i really like that idea of using those aria tags, which already carry a semantic meaning.
You can also simplify toggling the menu to:
function toggleMenu (isOpened) {
menuToggle.setAttribute('aria-expanded', `${!isOpened}`);
}
You may not even need a function for this case. If you use typescript, wrapping the boolean in a template literal is mandatory.
Instead of data-state data-is-opened can be used.
siteNavigation.dataset.isOpened= `${!isOpened}`;
If it haven't been said in the video the visibility: hidden can be used instead of display: block.
This is a fantastic video. Making sure our websites are accessible is such an important issue.
I think CSS property for .site-nav{visibility: hidden} perfect in this case. Because it supports CSS transitions and hides the ability to work with the keyboard. And also frees from lines in script.js
I may have missed something, if so, please correct me.
P.S.: I always admire open self-criticism on RUclips. Kevin, you are an amazing person and developer!
Elements set to {visibility: hidden} are removed from the accessibility tree, just like elements set to {display: none}. It would likely cause issues for screen reader users.
@@nilaallj our task is to hide navigation elements from the visibility tree when we need to hide this element. The display:none property didn't work due to missing animations. Why not use js instead visibility:hidden?
Was about to comment just that as well: Simply transition visibility one-way with a delay of the animation length or using steps(1) easing.
In some scenarios you might also want to toggle position to absolute, if the element being animated is in the layout flow and would leave an unintended hole.
This is just insane. I can see how solving invisible problems is important. But this is nuts. How many websites actually follow this kind of design?
I have learn something that I really have ignored all the time.
Thanks Kevin ❤️
Amount of knowledge I get from one video is unimaginable 🔥
This is an awesome video! Very informative, and the display:block solutions is a good one. I normally would use aria-hidden or remove all the tab-index on the links ;) This solution is much cleaner IMO thanks for sharing!
Kevin! I truly admire how you are passionate about CSS. Everytime I see your videos I learn something new. I wanted to ask, what are some projects that you have worked like websites that are out live today in the real world. You are ledengary.
Solid video! I just set up a navigation menu today, going to go through it tomorrow with all your points in mind. Thanks!
I've reviewed hundreds of job applications over the years, and the times I see front-end developers not having the faintest idea what W3C standards are is astounding. Sometimes I see them nesting and together, and similar nastiness. So many people simply don't understand that conflicting interactive elements shouldn't be nested. Many of them don't know there are rules to HTML, and many of them underestimate HTML. I mean, kudos to them for not ONLY using , but when you use semantics... use them correctly.
This is probably something that needs to be taught more, tbh. It's now 20+ years since I began to learn HTML and I was lucky enough to learn it from notes and literature that happened to talk at least a bit about a11y, but I get the sense most people that learn HTML in, say, university courses or whatever (1) just don't care that much about HTML in the first place and (2) don't really seem to ever get told that a11y is something you can improve in concrete ways if you write your code right.
The 'move fast and break things' culture that I see in a lot of startuppy Facebook-wannabe circles doesn't help either, of course.
@@narnigrin (1) - It is too easy to blame university courses or whatever as those have very limited time to actually teach those things.
(2) - Telling someone that writing code right will improve a11y won't magically make theme writing the code right ;)
I personally think that at the end of the day nothing really replace work experience and as long as people are willing to learn and peers are willing to share their experience, HTML semantics shouldn't be a deal breaker
I commented this, but comment has gone...
In this video you introduced a state-machine bug at around 13:30 and then do it more severe at 17:72 . Problem is not big enough in this particular case, but it’s very important when you introduce states and not carefully consider (and it’s easy to learn bad habits than fix these bad habits later on).
Later you introduced one more state and didn’t verify or check that previous state machine is actually correct.
At the beginning you had transitioning from „open” to „close” (and boolean type is a good enough to hold it), then you introduced „closing” state, but left check as boolean.
When doing ANY optimisation, it’s important to repeat loudly: „Preemptive optimisation is evil” and „premature optimisation is evil”. If you do optimisation, do automated (preferably) tests before AND after.
It’s very easy to reproduce if you click on menu button fast enough to close and open menu. The transitioning will be visually unpleasant and with a bit more complex state machine it’ll end up in an invalid state.
The correct transitioning will be check non-boolean state and do following in on-click event function:
* open -> closing
* closing -> closing (state changes from closing to close by an event)
* close -> open
For sake of completeness and to avoid visual flashes (when clicking fast enough), I’d add opening state. What if a clicker scenario like this: „click on menu button, press tab (to select first item), press space (to activate first item), press shift-tab (to select menu button) and press space (to activate menu button)”. I assume here that selected menu item changes content on screen without moving to another page.
To avoid such problems in the future, I recommend to create a function `menu_state(current_state)` which do all *state* transitioning and returning new state. On-click listener (in this case) will pass current state and get new one, if state has been changed, only then do a thing. This way a programmer can test state machine and communication with on-click separately.
PS: I wasn’t a guy who did a comment you mentioned, but I agree with it
PPS: It's better not to permanently delete a video, but add a link for correction to both the description and the pinned comment
JavaScript Improvements 2: A one-liner to toggle the aria-expanded property (two lines including the isOpen variable declaration)
menuToggle.setAttribute(isOpen ? "false" : "true");
No need for extra functions, just one line :D
or simply:
menuToggle.setAttribute((!isOpen).toString());
Or just menuToggle.setAttribute("" + !isOpen);
@@theweirddev could mess up older browsers tho
@@yolocat-dev Why?
That's what I was looking for. Bootstrap use this technique in their accordion. Expand -> Expanding -> Expanded.
My preference when it comes to programming is to go against the convention as a means of security through obscurity, on top of actual security standards. So using the information in the beginning of this video I’ve implemented a custom event that can be added to div buttons and instead of adding it with addEventListener you add it with prepareEventListener and based on which event you pass it, in this case it’s “sitename.interaction” it will set a tab index that’s based on its depth in the dom from body and use it’s innertext node to give it a aria-name property if it doesn’t already have one. And if you pass in a controlled child as an optional parameter it will ensure through setting a custom property if the child hasn’t been prepared or remanipulating it’s tabindex and it’s childrens to ensure they are above the tab index of the parent button, so you can layout your html however you need for zindex purposes and still get desired accessibility features without using the standard button which can be scraped for by general purpose bad actor scripts. Oh and I named the event interaction because it listens for both mouse down and pointer down and whichever event fires first serves the callback assigned and the second event to fire recognizes it’s already being handled which allows for dev tools to switch between emulating a pointer device and normal site without refreshing the page to reinitialize the event listeners based on the user agent or screen size as a fallback
Please make more mistakes, this taught me a lot! 🙃
18:55 would pointer-events: none work instead of display none for the tabbing issue?
Assuming not or you prob would have used. I’m also confused why it’s not setting that data state of closed ie display none when the animation to open it runs. Is it just the order set up?
So you click the button to open, animation runs and sets aria expanded to true , and data state to opened. Then you click again and sets to false, and closing, that animation runs and that’s what is targeted to listen for the ending. Guess that makes sense just kinda had to talk myself through it. That’s why it’s helpful to have in separate functions
Styling on aria attributes is something I do more and more: it forces you to add accessibility in the code which is a good thing
Stunning, wanna see more javaScript videos from you.
Accessibility is still a mystery to me, pls could we have a vid on that.
Things like the aria, keyboard accessibility, role, building accessible dropdowns, select, popups, modals etc.
Would be so grateful 🙏
OMG I've made this mistake soo much, I am changing everything RIGHT NOW!
Haha!
Good things should soon come to you for doing this. Keep it up Kevin! You should do a summary of interesting not obvious behaviors associated with pure HTML tags and attributes. I bet there is a lot there that developers should be aware of...
Love your videos Kevin, your newsletter and especially appreciate you revisiting old mistakes to improve accessibility!
However, the JS part contains a mistake so I am writing this comment hoping to clear up future viewers questions.
The isOpened being a string check is not a problem, most programming languages use enums or strings as checks for conditions.
Making it a boolean by moving the check in the assignment is a good idea, because the code is more readable and it prevents the confusion with an
if(isOpened===true)
situation.
The if around the ternary operator is a mistake, because the ternary operator evaluations work fine without it.
if(test?doA():doB()); // mistake, the if does nothing
test?doA():doB(); // basically a shorthand if else
And regarding people complaining that your JS is not concise enough, and could be shorter, I disagree with their point.
You make great educational videos, and your target audience is not a FE dev in typescript/React, writing ternaries and closures for minimum code size/maximum efficiency.
There are even transpilers, compressors and browser optimizations which can figure out your if/else can be turned into a ternary.
I can do concise and as a bonus create a reusable function for toggling:
const toggleAttribute = (obj, att) => ((obj, att, valA, valB) => obj[att] = (obj[att] === valA ? valB : valA))(obj, att, 'true', 'false');
toggleAttribute (menuToggle, 'aria-expanded');
But I would not write that one-liner both for my future self and future people working on the project.
In regards to fixing the mistake, I think the rest of the video is of too great value and a pinned comment just explaining that the ternary can stand on it's own is enough.
Don't stress yourself out, over a line of code.
Thank you Kevin for your videos and your newsletter, you're down to earth explanations brighten my mornings! 😃
At 17:50 i think the if useless because you did not open the bracket of the if statement and you can just use the ternary
25:00 Why don't you just transition `visibility` along with `clip-path`? It functions the same as `display: none` (at least when you position the element absolutely) and doesn't require the `@keyframes` trick or the animation event listener, nor the toggling of classes.
to preventing tabing through li items i guess, correct me if i am wrong.
I tend to avoid visibility since it takes up space still, but yes, since it's absolutely positioned it could have been a little bit easier, and still removes it from the accessibility tree.
@@KevinPowell Why didn't you add it straight into the ClipPathCirlcleClose, showed at 25:50 instead of all those sketchy shenanigans?
Hi Kevin! Vid could use a title addition. It should definitely have more engagement :) Awesome. Thans you addressed it this way
Just something to note at 17:50, you don't need to wrap the ternary operator in another if statement.
Instead of: if(isOpen ? closeMenu() : openMenu())
Just write: isOpen ? closeMenu() : openMenu()
The ternary operator is just a shorthand way of writing an if-else statement.
A ? B : C
A's truthiness is checked.
B is executed if A is truthy.
C is executed if A is falsy.
If you have no "else" condition then you can just do "A && B", where A's truthiness is checked and B is executed if A is truthy.
Очень крутой материал и не менее крутая подача материала!!! Кевин, смотрю Вас из России, Вы - лучший!!!
I remember doing that… add “cursor: pointer”, add “tabindex” to make it focusable, etc. No need to do much of that these days w/ first class support. Not to mention the impact on screen readers and other assistive tech with the new tags, it really cuts down on effort all around too.
Still waiting for the SASS course, Kevin 😅😅
As a relatively new frontend dev, I've encountered literally _all_ of those things and dealt with them in the _exact_ same way you did. Thanks for inspiring some confidence!
Thanks for the video and idea! 👍One question. Why create data-state="closing" when you could move the `display: none` property inside clipPathCircleClose for a value of 100% ?
aria-controls was only one I didn't know, but... It's cool I learned it after 16y in front end!
This is already an Amazing series
amazing video i didn't know about any of this. time to go back to my site and redesign and redevelop a little bit.
Well explained. Thanks for the all the details that are important!
thanks for your awesome demo, i actually had that this problem :-D, now i can fix my menu the proper way :-D
good job! a video about aria-labels would be much appriciated. like the most important ones people dont use!
I find what you can do with CSS fascinating, not just you, but like CSS peeps in general can do wondrous things. As a javascript guy though it hurt a little bit to see you put the ternary operator inside an "if()", heh. The "?" and ":" are the operators so they don't need to be wrapped up inside another block (especially an if).