Yes I get that. If you look at bootstrap navigation, it seems to prefer that approach as well.
2 Likes
Hi guys,
Thank you very much for your responses and for your time.
I tried to understand what you are saying but I am not sure if I made the necessary changes correctly or not?
So I deleted all the codes in the photo of post 22 and replaced it with this code:
#mainnav .menu > li.deeper.show > ul {
left: auto;
margin-top: 55px;
position: absolute;
}
But where should I put that JavaScript code?
I was not sure, so I created a new file called show.js and copied the following contents into the file
const dropdowns = document.querySelectorAll('.dropdown')
// close all excluding currDropdown
const closeAll = (currDropdown) => {
const dropdowns = document.querySelectorAll('.dropdown')
dropdowns.forEach((dropdown) => {
if (dropdown !== currDropdown) {
dropdown.classList.remove('show')
}
})
}
dropdowns.forEach((dropdown) => {
dropdown.addEventListener('click', (event) => {
const dropdown = event.target // element clicked on
// if element is child of dropdown ignore and return
if (!dropdown.matches('.dropdown')) return
// close any other open dropdowns first
closeAll(dropdown)
dropdown.classList.toggle('show')
})
})
document.addEventListener('click', (event) => {
const target = event.target
// if not clicking on a menu item close all menus
if (!target.closest('.main-nav li')) {
closeAll()
}
})
After that, I copied the show.js file in the /templates/js folder and added the following line to the index.php file.
<script type="text/javascript" src="<?php echo $tpljspath ?>/show.js"></script>
Maybe Iâm doing this wrong? Because as you can see on the site, the menu does not work
The code form @rpg_digital was using a class called dropdown but you have a class called .deeper so you would need to change to that class. Also as you are using anchors I think it better to target the anchor rather than the list item.
The code would then look like now.
const ddwns = document.querySelectorAll(".deeper > a");
// close all excluding currDropdown
const close = (currDropdown) => {
ddwns.forEach((dropdown) => {
if (dropdown !== currDropdown) {
dropdown.closest('.deeper').classList.remove("show");
}
});
};
ddwns.forEach((dropdown) => {
dropdown.addEventListener("click", (event) => {
const dropdown = event.target; // element clicked on
// close any other open dropdowns first
close(dropdown);
dropdown.closest('.deeper').classList.toggle("show");
});
});
document.addEventListener("click", (event) => {
const target = event.target;
// if not clicking on a menu item close all menus
if (!target.closest(".deeper")) {
close();
}
});
Note that the code is expecting no hover menu at all so you still need to remove this hover rule from your css.
Change the pseudo class :hover to a class of .show instead (etc⌠> li.deeper.show > ul).
It also looks like you deleted the small screen right:0 somewhere along the way. Itâs the second block of code in my image in post #21 for the 719px small screen media query.
#mainnav .menu li.deeper.show ul{
right:0;
left:0;
top:auto;
}
You didnât need to remove the :hover rules but instead change them to classes.
e.g. everywhere that you had li.deeper:hover you would change to li.deeper.show.
It also looks like like you have an old JS routine adding a .hover class to the menu but it adds it ok but wonât remove it so therefore I would remove any css that says .hover also. (li.deeeper.hover).
Iâve tested as best I can with those changes injected locally and the menu works for me as a click menu.
You do need to address all those issues correctly for this to work.
This is a very rough codepen and I have just extracted 3 css files and the menu to show that it can work.
Ignore the missing images and colours as this justy demonstrates the click action.
1 Like
Hi Paul,
Thank you very much for your helps.
I tried to do exactly what you said and made the changes exactly based on the file you posted, but it still doesnât work for me. In the second attempt, I tried to search even more and wherever I saw â:hoverâ for menu, I changed it to â.showâ, but it still doesnât work for me.
The problem is that the menu does not open at all.
There is no way to figure out where I went wrong? If possible, please take another look at the code and see where my mistake was.
Where did you put the script for it?
Thereâs too many files for me to look through.
1 Like
I found it 
<!-- MAINNAV -->
<script type="text/javascript" src="/templates/ja_wall/js/show.js"></script>
<div id="mainnav" class="has-toggle" dir="rtl" direction="rtl" >
You have placed the script above the menu html in the source which means the menu doesnât exist when the code is run. The js needs to be after the html that it references.
The best place for scripts is before the closing body tag.
Please try moving it and then try again
2 Likes
Hi Paul,
You are amazing. It works perfectly now. Exactly as I wanted.
Also Is it possible when I clicked/touched the menu item for second time to close it, the blue color of button return to normal (white)?
I think the following code is responsible for that color change:
#mainnav .menu > li:hover > a, #mainnav .menu > li > a:hover, #mainnav .menu > li > a:active, #mainnav .menu > li > a:focus {
background: url(../images/blue-tran-8.png) repeat;
border-bottom: 0;
color: #fff;
text-decoration: none;
Yes, thatâs the code that makes the change of color but the problem is whether you want the anchors to change on hover or not?
There is also the issue of :focus and once you click a link the :focus will be activated and the button will stay lit up until you click somewhere else ( because thatâs how :focus works).
I suggest that you move the code you posted above into a hover media query and just give it to hover devices. Then do the opposite for non hover devices.
Play around with something like this:
@media (any-hover: hover) {
#mainnav .menu > li:hover > a,
#mainnav .menu > li > a:hover,
#mainnav .menu > li > a:active {
background: url(../images/blue-tran-8.png) repeat;
border-bottom: 0;
color: #fff;
text-decoration: none;
}
#mainnav .menu > li > a:focus{
outline:1px dashed #ccc;
}
}
@media (any-hover: none) {
#mainnav .menu > li.show > a {
background: url(../images/blue-tran-8.png) repeat;
border-bottom: 0;
color: #fff;
text-decoration: none;
}
}
1 Like
Another new one to me Paul
1 Like
Hello Paul,
but the problem is whether you want the anchors to change on hover or not?
Honestly, I didnât understand exactly what you meant by this sentence.
For menus links, I think using javascript:void(0); Can it be better instead of #?Did you mean this?
Also, Iâd like to keep the hover as itâs currently set for display on PC, and touch/click for phone. (I made those settings based on the screen size)
I have implemented your last code, but as you can see on the site, the color of my text is having a problem. I tried to apply several different modes, but I couldnât fix it. Any suggestion to fix it?
Isnât there a css command to return the menu to the previous state exactly after the second click?
No and I donât like javascript mixed in the html.
What I was saying is that you canât hover an element as that will be treated as a first touch by devices and even if you changed the color on second click the element would still be focussed and hovered so would get the hover color.
Thatâs why I split the hover styles into devices that can hover so that touch devices do not receive them. The touch devices will get the click style.
I also removed the focus styles from the comma separated list because you had focus as white which means you wonât see it on a white background.
You have this.
#mainnav .menu > li:hover > a, #mainnav .menu > li > a:hover, #mainnav .menu > li > a:active, #mainnav .menu > li > a:focus {
border-bottom-color: #666;
color: #FFF;
text-decoration: none;
}
Thatâs not the same as I gave you above because you included the focus but I put it separate. However it seems you have left the original in place and not put it inside the any:hover media query as per my previous post.
This is what you have.
You need to delete that code and implement the code in my last post. If you get it right it will work as I have tested locally
1 Like
What I did before was that I copied the code you gave me in post #30 and replaced it with the code I mentioned in my post #29.( in theme.css file)
You need to delete that code and implement the code in my last post.
I think I already did that! And this is very strange because I canât find the code that you showed me in the attached photo of âlayout-mobile.cssâ inside this file.
I canât find following code :
#mainnav .menu > li:hover > a, #mainnav .menu > li > a:hover, #mainnav .menu > li > a:active, #mainnav .menu > li > a:focus {
border-bottom-color: #666;
color: #FFF;
text-decoration: none;
}
Maybe you are seeing a cached version of the site? Please refresh with ctrl+f5 and check it again?
Itâs in navigation.css as shown in the screenshot in my last post.
Iâll take another look tomorrow and double check when I get back to my desktop.
The menu is working on mobile apart from that focus style being applied.
1 Like
Yes the styles are exactly where I said they were and here is another screenshot of your navigation.css.
This is on another computer as I am away from home so there are no cache issues here (although I disable cache by default when devtools are open anyway).
You need to follow the instructions from my previous posts and make sure that focus rule is only seen in the hover media and on its own line as shown in my example.
There seem to be multiple rules spread through theme, navigation and layout-mobile.css all having an impact on this menu. Itâs a nightmare to debug and to follow and to make changes. Usually if you are using a theme you just create a custom over-ride css and do all your changes in there so you only need to look in one place. Also splitting the mobile into its own mobile layout css is a bit overkill and once again makes it 100 times harder to debug and maintain unless you are an expert and strict with your coding.
In essence you need to ensure that any rules that have li:hover > ul or li:hover > a are isolated in the any-hover:hover media query.
@media (any-hover: hover) {
/* rules for hover go here */
}
Then you would duplicate those rules for touch devices in the other media query and replace the li:hover with the li.show class instead.
e.g.
@media (any-hover: none) {
#mainnav .menu > li.show > a {
}
#mainnav .menu > li.show > ul {
}
/* etc..*/
}
Try and familiarise yourself with chrome devtools as you can immediately see where all the styles are cascading from and where you may have missed them.
1 Like
Hi Paul,
Thank you very much for your detailed explanation and all the help you have given.
You were right. It was in the navigation.css file and I fixed it. The site now works correctly on mobile phones and computers on screens larger than 986 pixels.
I see only a small problem that if I reduce the size of the Chrome browser below 986 pixels on the computer, it changes the menu correctly and turns into the tablet display mode, but when I click on the menu button, its color does not change to blue. Any suggestions on this?
1 Like
I think you are missing this rule:
@media only screen and (max-width: 984px) {
#mainnav .menu > li.show > a {
background: url(../images/blue-tran-8.png) repeat;
border-bottom: 0;
color: #fff;
text-decoration: none;
}
}
1 Like
Finally, everything is fixed! I never thought that adjusting menu color would be so complicated.
Thank you very much, Paul. It is really beautiful that you help others without any expectations. You are amazing.
1 Like
This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.