Help with JS logic to close menu if search is clicked, or vice-versa
Hello everyone,
I am currently working on my own website and have stumbled upon an issue with the functionality of mobile menu and search toggles in the site header.
Here is the link to a pen of my current code: https://codepen.io/is_dhananjay/pen/yLWyYjK
If you resize the window below 768px, the mobile toggle will kick in, hiding the menu item list. Clicking the menu toggle will then display the menu item list. Good so far.
But while the menu is open, if I click on the search button, the search container will open over the menu container. On the reverse end, if you open the search first, and then open the menu without closing search first, it will open, but behind the menu container.
I am still very new to JS, and here's what I am trying to achieve:
- If the menu toggle is clicked, first check if the search is open, if it's open, close it and only then open the menu.
- Same goes for the search toggle. If the search toggle is clicked, first check if the menu is open, close it, and only then open the search.
I would be great if I could get some help.
16 Replies
theres a few ways to approach this. one of the easier ways may be when either is clicked, run a function that looks for the
data-visible attribute , if its only being set on the menu and the search , you could run a querySelectorAll('[data-visible] . if its being used elsewhere, look on those elements specifically. then forEach, remove the attribute. once its removed you can add it back to the target.That seems apt. The attribute is only being used by the menu and the search containers.
Would it possible for you to spare some time and share some sample code? Sorry, I am a bit of a novice to JS.
One more thing, I will also need to set the
aria-expanded value for either to false first.
I mean the for the button toggles.
@clevermissfox While I was waiting for an initial reply, I also came up with some logic, and it does seem to work. I have updated the code on the pen. Can you please check?Instead if using two attr, why not use aria-expanded as your check? Especially if it needs to be toggled too
I'll look yes
You're on the right track with the logic you have but one thing is that when setting attributes like when you set Aria expanded to false it doesn't actually toggle like a Boolean, its just a string "aria-expanded", "false" vs "aria-expanded", false.
You'll just want to remove the whole attribute aria-expanded instead of setting it to false. You don't want screen reader users to have to go through a bunch of false attributes that don't apply (I think, im not great w a11y). If it's not applicable why waste their time. Then you can check whether the attribute exists there at all.
You mean, using
aria-expanded on both the toggles and the container?
Remove aria-expanded from where? The toggle buttons?something like this, i dont have much time to refactor everything but this is one option. how you have it working is also fine although a tad verbose. https://codepen.io/Miss-Fox/pen/rNgaoXa?editors=0010
also to check in an
if statement, you dont need to explicitly say if(x === true) or if(x === false). that can be shortened to if(x) meaning does x return true or if(!x) meaning if x DOESNT return true i.e. if x is false
one more thing before i sign off , the title tag belongs in the head of your document. <title>Toggle main menu</title> is not doing anything. in the head it gives the title of the webpage next to the favicon in the browser tab . you can use a title attribute in the html as you have but the title tag is for the head.
if you have any more questions ill be around later or there are lots of talented + brilliant people in this server that can help you or answer questions on thisThanks a lot for all the input. The code works great too!
As for the
<title> element, it's not the document title element. It's the SVG accessible name element: https://developer.mozilla.org/en-US/docs/Web/SVG/Element/title.
In the meanwhile, I will rework the JS, as I would need to conditionally add/remove the no-scroll utility class to/from the body element.Wouldn’t it need to be inside the svg to be associated though? it’s just the first child of the button before the svg is opened .
You could also take advantage of the :has() selector to not have to even deal with that in the js. Im bot really sure what the use case is for no-scroll (which I thought did something else given the name; I would rename to something that references the property for the utility class)
You could try something like
body:has(x:not([aria-expanded])) {overflow:hidden;} (or whatever selector you need; I’m not sure when you’re adding and removing it as I’m off the pc)
If you end up having more content on the page though, you would probably just want a different solution so you don’t have to set overflow hidden on the whole body for one element but you’re doing great where you’re at! You figured out the js on your own and made it work!Oh, I think, I messed up the title part when I was copying the code from the project to Codepen.
About using the
:has selector, that seems like a good idea. And if I end up still using the utility class, I would think of a better name.
And thank you so much for all the help and kind words. You were of great help! I am glad I joined this community. I learnt quite a few things about JS today.
Will go ahead and work on the final solution. I want to post it before I mark this thread as solved.
Oh, and wait, I really had <title> as direct child element of the <button> and not the <svg>. Close save! Would have been a big mess, huge thanks!No rush on solved, glad to help !
Maybe your next challenge could be animating the items as they slide off screen! You don’t get a smooth transition to display: none so there’s some hacks to make it work. I know KPow has a video about it too, I think it’s called something like “animating to display : none “ if you’re interested
I actually previously implemented the animation that ways. Instead of using
display property, I was using the transform which allowed me to animate the slide-out behavior. But that had it's own problems, like the animation appearing when resizing the window. That's exactly when I joined this community. Fortunately, i was able to fix it. Thanks to Steven: https://stevenwoodson.com/blog/solving-animation-layout-flickering-caused-by-css-transitions/
Anyways, I will check out the video by Kevin, animating to display: none would be soo cool 😉Steven Woodson - Independent Web Development Consultant
Solving Animation Layout Flickering Caused by CSS Transitions
Solving layout flickering (or animation flashing) happening on browser resize caused by CSS transitions, with live before and after examples!
Hi @clevermissfox, finally did it. Check out: https://codepen.io/is_dhananjay/pen/yLWyYjK
Yet to rename that
no-scroll utility class, but apart from that, everything now works as intended.
I added an additional check to make sure that the DOM has fully loaded. I got this idea from a stackoverflow post while I was searching for alternate solutions. Do you think it is necessary? I am already deferring the script and using priority hints when the script is called.DOM content loaded is not necessary if you are using the defer attribute or type=module.
Looks like the js is working! Ah you've discovered the ternary operator too!
Also I have to correct myself from earlier, looks like you should not remove aria attributes when their value is false as it won't be read out to screen readers and removing degrades the semantics so you're doing it right in that sense!
Sure. I removed that.
Yes, this was really fun learning. I am sort of proud of the code, looks kinda professional 😄
a11ty is something I have started taking seriously. I want the website to be highly (if not equally) accessible by everyone.
@clevermissfox Thanks again for all the help! I enjoyed our discussion. Please let me know if you have a GitHub account, I would like to follow you there.
That’s a great goal, it’s also on my list this year to improve with a11y. It’s just so massive and I’ve always heard that no aria is better than bad aria, and trying to find answers on WAI can be so contradictory. They show you eg how to make an accordion for a11y then there’s a disclaimer that the method shouldn’t be used in prod? So I stick to semantic html structure and sprinkle in aria tags when I know for sure they are correct.
Where are you learning accessibility ? I was struggling to find comprehensive courses or education in video form as that’s how I learn best. My adhd cannot get through the many many docs.
As for GH , same handle clevermissfox
I just followed you on GH. Let's rather continue this conversation on messages, it's getting off-topic 😄
I am marking this thread as solved now. For anyone who stumbles upon the same issue, please checkout the Codepen link. I have added the final code there.
Kudos to @clevermissfox for guiding me. Glad to be a part of this amazing community!