Javascript onclick won´t work at first click

I´ve got this function:

    <script>
  function bookmark() {
    var element = document.getElementById("bookmark");

    if (element.innerHTML === "Bookmark") {
      element.classList.add("bookmarked");
      element.innerHTML = "Bookmarked";
    } else {
      element.classList.remove("bookmarked");
      element.innerHTML = "Bookmark";
    }
  }
</script>

And it works good, it toggles the class "bookmarked" and switch the text from "Bookmark" to "Bookmarked".

But the thing is that it won´t work at first click, and I don´t find it functional. Does anyone knows what am I missing?

      function bookmark() {
        var element = document.getElementById("bookmark");

        if (element.innerHTML === "Bookmark") {
          element.classList.add("bookmarked");
          element.innerHTML = "Bookmarked";
        } else {
          element.classList.remove("bookmarked");
          element.innerHTML = "Bookmark";
        }
      }
button {
  color: white;
  font-weight: bold;
  height: 56px;
  width: 204px;
  border-radius: 33.5px;
  border: 0px;
}

button:hover {
  cursor: pointer;
}

.bookmark {
  background-repeat: no-repeat;
  background-color: rgba(47, 47, 47, 0.05);
  color: #7a7a7a;
}

.bookmark:hover {
  cursor: pointer;
  background-repeat: no-repeat;
  background-color: rgba(47, 47, 47, 0.05);
}

.bookmarked,
.bookmarked:hover {
  cursor: pointer;
  background-repeat: no-repeat;
  background-color: rgba(47, 47, 47, 0.05);
}
<button class="float-end bookmark" id="bookmark" onclick="bookmark()">
          Bookmark
        </button>

Upvotes: 0

Views: 56

Answers (4)

Nishant S Vispute
Nishant S Vispute

Reputation: 765

First thing i notice is that you are selecting the HTML element inside the fn()

    function bookmark() {
        var element = document.getElementById("bookmark");
    }

what i think you should do is select the element first and then on the button click event run the subsequent function

here is a alternative to your way PS: you only need to update your script and nothing else

Step 1: Find the element by id Assuming that there is just one such element else you can do this using querySelectorAll as well

Step 2: Add Click eventListener to that element.

Step 3: Run your Fn() Code

<script>
var element = document.getElementById("bookmark");

if (element) {
  element.addEventListener("click", (e) => {
    if (e.innerHTML === "Bookmark") {
      e.classList.add("bookmarked");
      e.innerHTML = "Bookmarked";
    } else {
      e.classList.remove("bookmarked");
      e.innerHTML = "Bookmark";
    }
  });
}
</script>

Upvotes: 0

Ayaz
Ayaz

Reputation: 2121

Spaces are added before the button text "Bookmark". You can use the trim() function to remove the space then compare. Change the below line.

if (element.innerHTML.trim() === "Bookmark")

Upvotes: 1

Ivan Ponomarev
Ivan Ponomarev

Reputation: 46

Do not use "innerHTML" field for comparison. Use classList.contains() method. and check if you have the class assigned or not. https://developer.mozilla.org/docs/Web/API/Element/classList

Upvotes: 0

brk
brk

Reputation: 50291

Remove all the white space using trim()

function bookmark() {
  var element = document.getElementById("bookmark");
  if (element.innerHTML.trim() === "Bookmark") {
    element.classList.add("bookmarked");
    element.innerHTML = "Bookmarked";
  } else {
    element.classList.remove("bookmarked");
    element.innerHTML = "Bookmark";
  }
}
button {
  color: white;
  font-weight: bold;
  height: 56px;
  width: 204px;
  border-radius: 33.5px;
  border: 0px;
}

button:hover {
  cursor: pointer;
}

.bookmark {
  background-repeat: no-repeat;
  background-color: rgba(47, 47, 47, 0.05);
  color: #7a7a7a;
}

.bookmark:hover {
  cursor: pointer;
  background-repeat: no-repeat;
  background-color: rgba(47, 47, 47, 0.05);
}

.bookmarked,
.bookmarked:hover {
  cursor: pointer;
  background-repeat: no-repeat;
  background-color: rgba(47, 47, 47, 0.05);
}
<button class="float-end bookmark" id="bookmark" onclick="bookmark()">
          Bookmark
        </button>

Upvotes: 0

Related Questions