patternjavascriptMinor
Responsive jQuery restaurant menu system
Viewed 0 times
restaurantsystemresponsivemenujquery
Problem
I have a working restaurant menu system - as the user selects a menu it slides into view. The build is responsive, and I've used the 'Debounced Resize() jQuery Plugin'. However, in implementing the responsive code, I feel my code has become clunky, and it needs refactoring.
The hope here is that I'll receive some constructive criticism and some pointers on how to improve my code, which will in turn grow my knowledge base.
HTML
jQuery
```
$(window).smartresize(function () {
var $mmenu = $('.menu__main');
if ($mmenu.length > 0) {
var $mnav = $('.menu__nav'),
$mnav_a = $mnav.find('a'),
m = parseInt($mnav.outerHeight(true), 10),
$contain = $mmenu.closest('.menu-container'),
h = 0,
l = 0;
// check and store if .active element exists
var active = $mmenu.not(':first').is('.active');
// if it doesn't exists
if (!active) {
$mmenu.hide() // hide all
.removeClass('active') // remove the active class from any that have it
.first() // select the first only
.addClass('active') // give it the active class
.show(); // and show it
$mnav_a.eq(0).addClass('active');
$mmenu.each(function(z) {
var $this = $(this);
$this.css('height','auto');
$this.css({
zIndex: z+1,
position:
The hope here is that I'll receive some constructive criticism and some pointers on how to improve my code, which will in turn grow my knowledge base.
HTML
Menu Title 1
Menu Title 2
Menu Title 2
Menu Title 1
jQuery
```
$(window).smartresize(function () {
var $mmenu = $('.menu__main');
if ($mmenu.length > 0) {
var $mnav = $('.menu__nav'),
$mnav_a = $mnav.find('a'),
m = parseInt($mnav.outerHeight(true), 10),
$contain = $mmenu.closest('.menu-container'),
h = 0,
l = 0;
// check and store if .active element exists
var active = $mmenu.not(':first').is('.active');
// if it doesn't exists
if (!active) {
$mmenu.hide() // hide all
.removeClass('active') // remove the active class from any that have it
.first() // select the first only
.addClass('active') // give it the active class
.show(); // and show it
$mnav_a.eq(0).addClass('active');
$mmenu.each(function(z) {
var $this = $(this);
$this.css('height','auto');
$this.css({
zIndex: z+1,
position:
Solution
Some of your code is redundant. These two lines are mutually exclusive. Especially since you add "active" to the first menu item, which is the only one which would satisfy !active.
Makes me wonder if the "if (!active)" branch can be removed entirely.
Revised CodePen: http://codepen.io/anon/pen/FHfAi
if (!active) {
.removeClass('active') // remove the active class from any that have itMakes me wonder if the "if (!active)" branch can be removed entirely.
Revised CodePen: http://codepen.io/anon/pen/FHfAi
Code Snippets
if (!active) {
.removeClass('active') // remove the active class from any that have itContext
StackExchange Code Review Q#47220, answer score: 3
Revisions (0)
No revisions yet.