Welcome to the Treehouse Community
Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.
Looking to learn something new?
Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.
Start your free trial
Rossella Rosin
15,734 Pointsalternate solution to add 'selected' property to current page option
Just for the sake of it, I wanted to do the job outside of the 'each' loop. Maybe because that's how my brain works ;-), you know, I won't loop over the menu items, I would just "select the option that corresponds to the current page in the menu", in one step. Seems more intuitive to me.
so here it is:
// add 'selected' attribute to the option that has a value equal
// to the href of the anchor contained in ".selected" class
var currentPageHref = $(".selected a").attr("href");
var $currentPageOption = $("option[value = '" + currentPageHref+ "']");
$currentPageOption.prop("selected", true);
I suppose that from computer perspective the code in the video might be more efficient, but in any case, here's my alternate solution - it might be useful in the future perhaps. Just sharing ;-)
Comments and critiques welcome!
2 Answers
Steven Parker
229,732 PointsWhenever you can replace a loop entirely with a deterministic process, that's a "best practice" and improvement in efficiency. But unfortunately, that's not the case here. You need the loop to create the options anyway.
I suspect this change would actually reduce efficiency, since inside the loop you already have the $anchor and $option variables for the elements, but this way you have to perform two selections later to get them again. Either way, the property assignment is only going to be performed once, so you're eliminating a few tests on an already-selected $anchor with two new selections which both look through the entire document for matching elements.
Another issue with this replacement is that the loop only looks at the anchors that descend from the #menu element, but the independent selector would look for all elements descending from anything with the selected class. It makes no difference in this particular HTML document, but in general it could be an issue.
Also, the precise selector to replace $anchor.parent().hasClass("selected") would be ".selected > a". The descendant selector ".selected a" would target every anchor with anything having class selected in its ancestry, not just its parent.
So, combining these would make the first selector: "#menu .selected > a".
Now you can improve the efficiency of the second selection a good bit by using that $select variable that is still in scope outside the loop. This also solves the problem of the selector finding options belonging to a different SELECT statement otherwise. The modified line would look like this (you can drop "option" since all the select's children are options):
var $currentPageOption = $select.children("[value = '" + currentPageHref+ "']");
Overall ... good try, and I really like your efficiency-mindedness! Happy coding!
Graham Wright
Full Stack JavaScript Techdegree Student 15,197 PointsWhat about this alternative?
$select.val(window.location.pathname.slice(1));
Rossella Rosin
15,734 PointsI'd have to refresh my jQuery, because I don't understand very well what you are trying to extract with this. The jquery slice() method selects part of a set of objects, the javascript slice() method selects part of an array. Window.location.pathname is neither a set of objects nor an array, if I remember well, but just a string, so I suppose this code doesn't work, or just gets the pathname....?
In any case, I usually prefer to be free from window.location whenever creating navigation. Simply because sometimes the url doesn't reflect the navigation content. Pages are sometimes redirected or generated dinamically.
it's best to get the href from the link inside the page.
Steven Parker
229,732 PointsThere's also a JavaScript string slice method, which would be the one applied above. So it's just taking off the first character of the pathname.
I'd have to watch the video again, but I'm not sure that the current location path is guaranteed to be the same as the href of the option with the selected class - is it?
Rossella Rosin
15,734 PointsNo, Steven, it isn't. That's what I was trying to say in second part of my comment :-)
Rossella Rosin
15,734 PointsRossella Rosin
15,734 PointsThank you!! I suspected there was some efficiency reason behind the choice in the video. Otherwise Andrew would have implemented my solution instead :-)
And thanks a lot for improving my code bit!