Accordion Menu Problems

[ellenjones6]

Ah, my mistake! I see that when I tested out your revised script for making the accordions persistent, I cut and pasted an earlier version of your suggested script. I went back and am now using your latest streamlined version. (At least, I HOPE I am doing it correctly this time!)

I did make one change, to make the accordions work in IE9.

Instead of this line of code (from you latest streamlined version):

		this.classList.toggle("active");

I pasted this code, which you sent me yesterday:

    this.className = /\bactive\b/.test(this.className)? this.className.replace(/\bactive\b/, '') : this.className? this.className + ' active' : 'active';

With this change, which you had recommended in an earlier post, the accordions work in IE9 emulation. Of course, nothing works in IE8 or earlier, but I have decided not to worry about that.

I still get the Dreamweaver error messages (for both your latest streamlined script and that script modified with your suggested code to make it work in IE9):



If you tell me that these errors are not fatal, I am not going to worry about them.

As always, thank you so much for all the time you have spent helping me.

ellen

[jscheuer1]

OK, well I've looked into this further, and localStorage is a bit of a non starter because, if it's disabled, at least in some browsers, that will cause a fatal error for the entire script unless error checking is done that would add more bulk to the code than you probably want. So, I'm reverting to cookies. Here's the version I think we should go with, assuming you still want to support IE 9:

(function(){
	var accs = document.querySelectorAll('.accordion, .accordionSecond'),
	cookiename = 'accordionpersiting_73561',
	persisted = document.cookie.match('(^|;)\x20*' + cookiename + '=([^;]*)'),
	persisting = {}, i = accs.length;
	if(persisted && (persisted = persisted[2])){
		var p = persisted.split('.'), l = p.length;
		while(--l > -1){persisting[p[l]] = true;}
	}
	function clickfunction(){
		var panel = this.nextElementSibling;
		this.className = /\bactive\b/.test(this.className)? this.className.replace(/\bactive\b/, '') : this.className? this.className + ' active' : 'active';
		panel.style.maxHeight = panel.style.maxHeight? null : panel.scrollHeight + "px";
		persisting[this.getAttribute('data-index')] = !!panel.style.maxHeight;
	}
	while(--i > -1){
		accs[i].setAttribute('data-index', i);
		accs[i].addEventListener('click', clickfunction, false);
		persisting[i] && clickfunction.apply(accs[i]);
	}
	window.addEventListener('unload', function(){
		var pa = [], p;
		for(p in persisting){persisting.hasOwnProperty(p) && persisting[p] && pa.push(p);}
		document.cookie = cookiename + "=" + pa.join('.') + "; path=/";
	}, false);
})();

Now, if cookies are disabled in a given browser, the persistence will not work, but most folks have them turned on, and even if not, there will be no error. You should not be getting the same warnings in Dreamweaver with this version. If you do, it's still looking at the old code. This has no missing semi-colon (or if I missed one, it's no longer on line 23), nor does it have a function inside the loop, though it might. Technically I placed the function outside the loop and only reference it from inside it. But a strict script parser may see that as the same thing. The 'use strict' is something I'm not even all that familiar with. I do know it can be added to code and when you do it means that the script code must be interpreted in a certain way, however, it's interpreted just fine without it, and I'm not certain that without certain syntax changes that it still would be if it were added.

Later I will have a look at this IE rendering problem at page load that you mentioned.

[ellenjones6]

Many thanks! I have incorporated the new code you suggested. I still get error messages in Dreamweaver, but I am going to assume that they are not fatal errors. I have no clue how Dreamweaver comes up with these error messages. There is a list of linting rules here ( http://jshint.com/docs/options/ ), but I don't know how they relate to the file I am working with.

Anyway, as long as the errors are not fatal, I probably shouldn't worry about them. True?

Thanks again.

[jscheuer1]

Right, that is true. Even in the graphic you posted, they're listed as errors/warnings. They can't be both. These are just warnings. The semi-colon warning is useful if you're going to compress the code, but only sometimes, some semi-colons can be omitted. I looked into this "use strict" thing more, and think I actually like the idea now, but only for testing purposes. I tried it with the latest code and it passed. But using it in live code can make it take a little longer to parse. Out of curiosity, what warnings are you now getting other than the one about "use strict"?

[ellenjones6]

Line 2, Column 5 Missing "use strict" statement.
Line 19, Column 53 Expected an assignment or function call and instead saw an expression.
Line 23, Column 9 The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype.
Line 23, Column 88 Expected an assignment or function call and instead saw an expression.

[jscheuer1]

Thanks for posting those. Those are all nothing more than warnings. I already covered the "use strict" issue and this code passes that. As for the rest, I use shortcuts to eliminate excess coding. I use something && somethingelse(); to replace if(something){somethingelse();}

These warnings are because I eliminate the need for if and it's concomitant constructs where the checker might normally expect that. And because I use the last condition as the resultant action should the rest be true, where the checker expects to see something further done. It thinks I'm saying:

if, if, if;

But I'm really saying:

if, if, do();

As for this IE thing with the text being real big momentarily, I'm not sure, but think this highlighted style may be causing it (line 452 in stylesADKWild.css):

.asideLeft {
	float: right;
    background: #e7f0fd;
	width: 100%;
	font-size: 1.5em;



}

It's later contradicted (line 785) where the exact same selector has its font-size set to 1em, so you should be able to get rid of it without any problems. After doing so, clear the cache and reload the page(s). If that takes care of it, great! But, as I say, I cannot be sure if it's the problem. In any case, it can't hurt anything to get rid of it.

[ellenjones6]

Thanks! I got rid of the font-size on line 452. The weird text thing in IE with the text being momentarily larger is now gone.
That was easy!

Thanks again, ellen

[jscheuer1]

I thought about this some more, and I think it was also caused by the transition properties on the button.accordion and the button.accordionSecond selectors. Then, as the css loaded in IE, since the font size "changed", it had to transition. Other browsers didn't see this as a change because they parsed the stylesheet as a whole.

I also had a thought that it would be nice to skip the transition on the panel divisions when the page is navigated to or refreshed and persistence enabled. Doing that was a little trickier than I imagined, but not too bad once I got it. If you want to try this, add this in the stylesheet as shown:

div.panel {
	margin-top: .5em;
	margin-bottom: .5m;
	width: 25%;
    background-color: #ffffff;
	color: #000000;
    max-height: 0;
    overflow: hidden;
    transition: max-height 0.2s ease-out;
    font-family: "Times New Roman", Times, serif;
}
div.panel.notransition {
  transition: none !important;
}

And use this version of the script:

(function(){
	var accs = document.querySelectorAll('.accordion, .accordionSecond'),
	cookiename = 'accordionpersiting_73561',
	persisted = document.cookie.match('(^|;)\x20*' + cookiename + '=([^;]*)'),
	persisting = {}, i = accs.length, toggleClass = accs[0] && accs[0].classList? 
		function(el, name){el.classList.toggle(name);} : function(el, name){
			var classes = el.className.split(' '), i = classes.length, has = false;
			while(--i > -1){if(classes[i] === name){has = true; classes.splice(i, 1); break;}}
			!has && classes.push(name);
			el.className = classes.join(' ');	
		};
	if(persisted && (persisted = persisted[2])){
		var p = persisted.split('.'), l = p.length;
		while(--l > -1){persisting[p[l]] = true;}
	}
	function clickfunction(trans){
		var panel = this.nextElementSibling;
		toggleClass(this, 'active');
		if(trans === false){toggleClass(panel, 'notransition');}
		panel.style.maxHeight = panel.style.maxHeight? null : panel.scrollHeight + "px";
		if(trans === false){panel.offsetHeight; toggleClass(panel, 'notransition');}
		persisting[this.getAttribute('data-index')] = !!panel.style.maxHeight;
	}
	while(--i > -1){
		accs[i].setAttribute('data-index', i);
		accs[i].addEventListener('click', clickfunction, false);
		persisting[i] && clickfunction.apply(accs[i], [false]);
	}
	window.addEventListener('unload', function(){
		var pa = [], p;
		for(p in persisting){persisting.hasOwnProperty(p) && persisting[p] && pa.push(p);}
		document.cookie = cookiename + "=" + pa.join('.') + "; path=/";
	}, false);
})();

It includes enhanced backward compatibility for IE 9's lack of the classList.toggle() function, while allowing those browsers that do support it to use it.

[ellenjones6]

Many thanks!

I incorporated the revision to the css plus the revised script. All seems to be working fine on all browsers (with the exception, of course, of the iPad browsers -- the accordions work fine, but do not persist). But that's been true through all the changes. So, if it cannot be easily fixed, I am just not going to worry about it.

It may be just my imagination, but the movement from page to page seems to be somehow "steadier," in the sense that it feels like the left navigation menu is not changing, while the banner and new content of the new page flows in around it.

So, I am really happy with what we've ended up with. This morning, I tinkered with the styling to change the palette just a tad, in an effort to make the menu more bland and vanilla. The previous gray I was using had a bit too much blue, so I messed around with other grays to come up with something that I think looks more neutral, because it has to work with all sort of different colors in the banner and hero image.

Thanks again for all the time you have spent helping me.

Ellen

[jscheuer1]

Smoother page transitions was the purpose for the latest change I offered. It's not quite as good in some browsers, but better in all of them, and very smooth in some, most I think.

The persistence issue in iPad is almost certainly due to having cookies turned off, either in the OS, and/or in the affected browsers. It's possible that iPad and/or certain iPad browsers have cookies off by default*. In any case though, you said it worked in Opera mini on iPad, so the device is at least capable of storing cookies.

*Generally though, cookies enabled is default and recommended.