1

I would like for my function to return me the value of the string inside the button, to later be used in other stuff.

I tried (this).innerHTML and this.innerHTML to extract the string inside of the button and later be used in the js HTML:

<button class="fonts" value="2" onclick="change((this).innerHTML)">Times New Roman</button><br>
<button class="fonts" value="1" onclick="change(this.innerHTML)">Calibri</button><br>
<button class="fonts" value="3" onclick="change(this.innerHTML)">Arial</button>

JS:

function change(){
  alert();
}

I wanted it to alert "Times New Roman" or the other fonts, yet it alerted a blank value.

AndrewL64
  • 15,794
  • 8
  • 47
  • 79
Ali Chbinou
  • 93
  • 1
  • 11

4 Answers4

1

You are not actually passing the innerHTML into the function. The JS should look more like this:

function change(font){
    alert(font);
}

And the HTML would look something like this:

<button onclick="change(this.innerHTML)">Times New Roman</button>
<button onclick="change(this.innerHTML)">Calibri</button>
<button onclick="change(this.innerHTML)">Arial</button>

Here's a fiddle of this in action: JSFiddle


If you want an even simpler implementation, you could do something like this:

JS:

function change(e){
        e = e || window.event;
    e = e.target || e.srcElement;
    alert(e.innerHTML);
}

HTML:

<button onclick="change()">Times New Roman</button>
<button onclick="change()">Calibri</button>
<button onclick="change()">Arial</button>

JSFiddle Demo

hopkins-matt
  • 2,763
  • 2
  • 15
  • 23
  • Hello! The second one was not simpler at all :') I'm still learning in JS and that seemed pretty complicated x) However, I used the code you wrote earlier and added (font) between parentheses and it worked! Could you please explain why it did? – Ali Chbinou Jan 29 '19 at 21:48
1

You should avoid using inline on* handlers (onclick, oninput, etc) and instead use an event listener within your script itself for alerting the elements' text content.

Check and run the following Code Snippet for a practical example of what I have described above:

/* JavaScript */

var btns = document.querySelectorAll(".fonts"); // retrieve all buttons

//use forEach() to add a click event listener to each button
btns.forEach(function (btn) {
  btn.addEventListener("click", function() {
    alert(this.innerHTML);
  });
});
<!-- HTML -->

<button class="fonts" value="2">Times New Roman</button><br>
<button class="fonts" value="1">Calibri</button><br>
<button class="fonts" value="3">Arial</button>

You can further shorten the above code by using JavaScript ES6+ instead like this:

document.querySelectorAll(".fonts").forEach(btn => {
  btn.addEventListener("click", () => alert(btn.innerHTML));
});
<button class="fonts" value="2">Times New Roman</button><br><button class="fonts" value="1">Calibri</button><br><button class="fonts" value="3">Arial</button>

N.B. Do note that if you use the ES6+ approach, you will need to use a JavaScript compiler like Babel to convert your ES6+ syntax into a backwards compatible version of JavaScript on production to support current and older browsers or environments.


IE11 Compatibility:

If you are concerned with IE11 compatibility of the forEach() method, you can use a simple for loop instead like this:

/* JavaScript */

var btns = document.querySelectorAll(".fonts"); // retrieve all buttons

//use forEach() to add a click event listener to each button

for(i=0; i < btns.length; i++) {
  btns[i].addEventListener("click", function() {
    alert(this.innerHTML);
  });
}
<!-- HTML -->

<button class="fonts" value="2">Times New Roman</button><br>
<button class="fonts" value="1">Calibri</button><br>
<button class="fonts" value="3">Arial</button>
AndrewL64
  • 15,794
  • 8
  • 47
  • 79
  • Please note that `NodeList` doesn't support `forEach` in IE 11. – connexo Jan 29 '19 at 21:55
  • So first of all, I should avoid using on* handlers in HTML and rather use addEventListener instead from now on? I've seen addEventListener many times but I don't reall know how it works. I will document myself upon that – Ali Chbinou Jan 29 '19 at 21:55
  • Also thanks for your answer, I think I understand it (+/- for the forEach part) – Ali Chbinou Jan 29 '19 at 21:56
  • @ali Yes, event listeners should be used because it prevents the mixing up of HTML and JavaScript within the same file. – AndrewL64 Jan 29 '19 at 21:56
  • @AliChbinou If you are concerend with IE comptability issues, I have updated the answer with an IE11 compatible version too. Cheers. – AndrewL64 Jan 29 '19 at 22:02
  • @AndrewL64 you say _You should avoid using inline on* handlers_ but this against modern way of programming html template like in [angular](https://angular.io/guide/template-syntax) `(click)="handler(...)"` or [vue.js](https://vuejs.org/v2/guide/syntax.html) `@click="handler(...)"` . In my opinion it is not good to put whole logic/function body into `inline on`, but call a handler is allow and makes code more clear – Kamil Kiełczewski Jan 30 '19 at 03:15
  • @Kamil It is considered good practice to keep the html, css and js separate to make sure that your code structure is semantic **unless** one is using libraries or frameworks that would require the use of on* handlers. But that is somewhat obvious for cases where libraries or frameworks are used mate. Cheers. – AndrewL64 Jan 30 '19 at 07:49
  • 1
    @AndrewL64 but do you know WHY this good practise to NOT USE handlers in html, when in the same time it is good practise to USE handlers in html templates? Double standards? – Kamil Kiełczewski Jan 30 '19 at 07:54
  • 1
    @Kamil One of the main reason why it's bad practice is because inline handlers run the events on a global scope. You can check out the answers in this [SO thread](https://stackoverflow.com/questions/5871640/why-is-using-onclick-in-html-a-bad-practice) for a more in-depth explanation on that as well a highlighting other reasons for why it's a bad practice. Cheers mate :) – AndrewL64 Jan 30 '19 at 08:06
1

Using inline event listeners like onclick="whatever()" is widely considered bad practice.

Instead, get a collection of your buttons from the DOM using document.querySelectorAll(selector), and use addEventListener() on each button in a for...of loop:

let buttons = document.querySelectorAll('button.fonts')

for (const button of buttons) {
  button.addEventListener('click', function() { alert(this.textContent); })
}
<button class="fonts" value="2">Times New Roman</button><br>
<button class="fonts" value="1">Calibri</button><br>
<button class="fonts" value="3">Arial</button>
connexo
  • 53,704
  • 14
  • 91
  • 128
-1

Try

function change(btn) {
  alert(btn.innerHTML);
}
<button onclick="change(this)">Times New Roman</button>
<button onclick="change(this)">Calibri</button>
<button onclick="change(this)">Arial</button>
MikeM
  • 13,156
  • 2
  • 34
  • 47
Kamil Kiełczewski
  • 85,173
  • 29
  • 368
  • 345
  • @AliChbinou this works because I just pass button (`this`) into `change` function and finally its `innerHTML` into alert function (your alert function in question was empty so it not show anything). It should also works if you pass text in this way in html `onclick="change(this.innerHTML)">` and `function change(text) { alert(text); }`. However my solution in one hand not force you to duplicate `this.innerHTML` 3 times in html and in the other hand is clear and simple. – Kamil Kiełczewski Jan 30 '19 at 03:03
  • @AliChbinou You also don't need to use `addEventListener` instead `onclick`, you can do it like I which is similar to use e.g. angular/vue.js templates. So you can call JS function in html template, however you should not put whole function body to html. – Kamil Kiełczewski Jan 30 '19 at 03:09