HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Display or hide drawer with onClick

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withdrawerdisplayhideonclick

Problem

The following is my code for my "drawer" to be display or hidden on the basis whether it has been clicked or not. I want to make this better as personally its messy. Any tips?

//Checks if the page loaded is of specific type    
if(window.location.href.indexOf("home.php") > -1) {
      //Handles the displaying of the side bar on the home page.
      var button = document.getElementById('navIcon');
      button.onclick = function() {
          var div = document.getElementById('drawer');
          if (div.style.display !== 'block') {
              div.style.display = 'block';
              //If the document is over 900px. Readjusts the grid, Otherwise overlay
              if(document.body.clientWidth > 900) {
                  document.getElementById('resGrid').style.width = 'calc(100% - 240px)';
              }
          }
          else {
              div.style.display = 'none';
              document.getElementById('resGrid').style.width = '100%';
          }
      };
    }

Solution

button.onclick = function() {


Use addEventListener instead. That way, you can add as many handlers as you want without replacing any previous ones assigned.

if (div.style.display !== 'block') {


Instead of using inline CSS, put them in stylesheets and define the styles in classes. Then have JS assign/remove classes instead to apply/remove. The problem is that 1) You have CSS in your JS and 2) inline CSS have very high specificity. The only way to override them is to replace the inline style value, or use !important on the stylesheet.

if(document.body.clientWidth > 900) {


If you also use CSS, this can easily be remedied by media queries.

if(window.location.href.indexOf("home.php") > -1) {


You probably want to use a more robust routing library like Page.js to do this.

Code Snippets

button.onclick = function() {
if (div.style.display !== 'block') {
if(document.body.clientWidth > 900) {
if(window.location.href.indexOf("home.php") > -1) {

Context

StackExchange Code Review Q#123485, answer score: 5

Revisions (0)

No revisions yet.