javascript display 'none' not executing-CodePudding

I'm trying to make less pages for my application so I decided to put everything in one HTML and do the visibility with JS. Now I got 2 of the 4 functions working but for some reason the function: extra() & settings() are not working. These 2 function do get executed but instead of showing the content of the div that belongs to the function I get a blank page and no content. Am I overlooking something here? because to me it exactly the same as the other functions which do work.

// Default
document.getElementById("information").style.display = "none";
document.getElementById("settings").style.display = "none";
document.getElementById("extra").style.display = "none";

function index() {
  document.getElementById("index").style.display = "block";

  document.getElementById("information").style.display = "none";
  document.getElementById("settings").style.display = "none";
  document.getElementById("extra").style.display = "none";
}

function information() {
  document.getElementById("information").style.display = "block";

  document.getElementById("index").style.display = "none";
  document.getElementById("settings").style.display = "none";
  document.getElementById("extra").style.display = "none";
}

function settings() {
  document.getElementById("settings").style.display = "block";

  document.getElementById("information").style.display = "none";
  document.getElementById("index").style.display = "none";
  document.getElementById("extra").style.display = "none";
}

function extra() {
  document.getElementById("extra").style.display = "block";

  document.getElementById("information").style.display = "none";
  document.getElementById("settings").style.display = "none";
  document.getElementById("index").style.display = "none";
}

<div id="index">
  <a  onclick="settings()">settings</a>
  <a  onclick="information()">information</a>
  <a  onclick="extra()">extra</a>
</div>

<div id="information">
  <p>this is a text</p>
</div>

<div id="settings">
  <p>this is a text</p>
</div>

<div id="extra">
  <p>this is a text</p>
</div>

CodePudding user response：

You should rethink how you are showing and hiding the elements and make your code more friendly to change in the future. Say you want to add another element, you have to add it to every function call.

What you need to do is build it so all the common stuff is in one function. This uses classes instead of setting the style directly.

// function that will show and hide the data
function toggleContent(selector) {
  if (!selector) return;
  // find the active wrapper and hide it
  const activeWrapper = document.querySelector(".wrapper.active");
  if (activeWrapper) activeWrapper.classList.remove('active');
  
  // Select the element that now is active and display it
  const currentWrapper = document.querySelector(selector);
  if (currentWrapper) currentWrapper.classList.add("active");
}

// function called by the index wrapper when clicked
// Using event delegation so we do not have to add multiple event handlers
function toggleTriggered (evt) {
  // find the anchor that was clicked
  const anchor = evt.target.closest("a");
  // If they did not click an achor, ignore it
  if (!anchor) return;

  // get the data attribute so we know what to show
  const toggles = anchor.dataset.toggles;
  
  // Show the content
  toggleContent(toggles);
}

// Add a single click listener to the menu
document.querySelector("#index").addEventListener("click", toggleTriggered);

// set the default state
toggleContent('#index');

.wrapper {
  display: none;
}

.wrapper.active {
  display: block;
}

<div id="index" >
  <a  data-toggles="#settings">settings</a>
  <a  data-toggles="#information">information</a>
  <a  data-toggles="#extra">extra</a>
</div>

<div id="information" >
  <p>information</p>
</div>

<div id="settings" >
  <p>settings</p>
</div>

<div id="extra" >
  <p>extra</p>
</div>

CodePudding user response：

Let's try and simplify your code to make it as DRY as possible. At the moment you're repeating lines of code that can be encapsulated far better.

But to do that I need to introduce you to a few new concepts:

Event delegation. We wrap a container element around the buttons, and add a click listener to it. That listener captures all the events from its child elements, and then calls a handler function (handleClick in this case). We no longer need listeners on each button. It's also useful if you want to programmatically add a new button to it. That button will automatically work.
querySelector, and querySelectorAll
classList because we're not going to be using any inline styling.
Destructuring assignment
Data attributes
Template/string literals
addEventListener so that we're not using inline JavaScript any more.

It's a lot - but it's very useful, and as a JavaScript developer you're going to run into these concepts a lot.

We add data attributes to both the buttons and the "panels" so they match, and add a panel class to the "panel". When the container captures an event from a button it grabs the node name from the clicked element (so we can check it's a button), and the id from the dataset.

If it's a button, we iterate over all the panel elements and remove the "show" class. And then we grab the element with the data attribute that matches the id from the button, and add the "show" class.

// Cache your elements
// querySelector/querySelectorAll
const container = document.querySelector('.container');
const panels = document.querySelectorAll('.panel');

// Add a listener to your button container
container.addEventListener('click', handleClick, false);

function handleClick(e) {

  // Destructure the nodeName, and id from the clicked
  // element's dataset - basically `e.target` returns an
  // object with a nodeName property, and a dataset property
  // so we extract the nodeName, but also the id from the dataset
  // which is also an object
  const { nodeName, dataset: { id } } = e.target;

  // Doublecheck that the element that was clicked on
  // was a button. We don't want to act on any other elements
  // in the container that may have been clicked on
  if (nodeName === 'BUTTON') {

    // For each panel remove the show class
    panels.forEach(panel => panel.classList.remove('show'));

    // Find the panel with the data id that matches
    // the id from the button - this is where we use
    // the template string
    const panel = document.querySelector(`.panel[data-id="${id}"]`)

    // Add the show class to it
    panel.classList.add('show');

  }

}

.panel { display: none; }
.show { display: block; }

<div >
  <button data-id="settings">Settings</button>
  <button data-id="information">Information</button>
  <button data-id="extra">Extra!</button>
</div>

<div  data-id="information">
  <p>This is information</p>
</div>

<div  data-id="settings">
  <p>This is settings</p>
</div>

<div  data-id="extra">
  <p>This is extra!</p>
</div>