Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Switch with Pulling from Map #3

Open
Ellisande opened this issue Mar 2, 2017 · 1 comment
Open

Replace Switch with Pulling from Map #3

Ellisande opened this issue Mar 2, 2017 · 1 comment

Comments

@Ellisande
Copy link

When adding the class for reveal you can replace the switch statement since you already have a map declared.

      switch (settings.reveal) {
        case 'bottom':
          $(settings.panel).addClass(classes.bottom);
          break;
        case 'left':
          $(settings.panel).addClass(classes.left);
          break;
        case 'right':
          $(settings.panel).addClass(classes.right);
          break;
        case 'fade':
          $(settings.panel).addClass(classes.fade);
          break;
        default:
          $(settings.panel).addClass(classes.top);
          break;
      }

can easily be re-written as

   $(settings.panel).addClass(classes[settings.reveal] || classes.top)

Reduces the lines of code and get rid of the often problematic break; requirement between cases.

https://github.com/bebaps/clear-menu/blob/master/src/clearmenu.js#L32

@bebaps
Copy link
Owner

bebaps commented Mar 2, 2017

That's a good point, and I never thought of it that way. Will implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants