2
\$\begingroup\$

I have the following code which return some text in different fonts (for the moment only TimesNewRoman and Arial). It works fine but if I want to make it work for 20 or more fonts (but not necessarily font, also for font-size, or color, etc) I would have to declare a ton of variable and a very long Js code. Not to speak about CSS code.

I am wondering if there is another way to do this in an easier way and more closed to a deliverable code.

var field, theDivTNR, theDivA;

function init() {
  console.log("page loaded and DOM is ready");
  field = document.querySelector("#inputField");
  theDivTNR = document.querySelector("#theDivTNR");
  theDivA = document.querySelector("#theDivA");
}

// The next function is called each type a key has been
// typed in the input field
function showWhatITyped() {
  // fill the div with the content of the input field
  theDivTNR.innerHTML = field.value;
  theDivA.innerHTML = field.value;
}
#theDivTNR{
	font-family: timesnewroman;
	font-size: 20px;
}
#theDivA{
	font-family: arial;
	font-size: 20px;
}
<html>
<head>
	<title>JavaScript Course</title>
	<script type="text/javascript" src="script.js"></script>
	<link rel="stylesheet" type="text/css" href="style.css">
</head>
<body onload="init();">
	<p>Type text here: <input type="text" oninput= "showWhatITyped();" id="inputField" ></p>
	<p>What you typed:</p>
	<p>TimesNewRoman</p>
	<div id="theDivTNR"></div>
	<p>Arial</p>
	<div id="theDivA"></div>	

</body>
</html>

| improve this question | |
\$\endgroup\$
1
\$\begingroup\$

You want the each font name to be listed just once. You could put them in either the JavaScript, the CSS, or the HTML. I recommend putting them in the HTML, since you actually want them to be displayed. The least verbose way that I can think of is to use a title attribute on each <div>, then make the attribute visible using CSS.

Instead of .innerHTML, I think you should use .innerText. The difference is in how it behaves in response to HTML tags (such as <i>) and entities (such as &amp;) that are typed into the text field. If your prompt says to type text (not "Type HTML here"), and you use a one-line text field (instead of a <textarea>), then I would expect it to behave like text rather than HTML.

It is better modern practice to avoid littering your HTML code with JavaScript hooks. Rather, the JavaScript code should register itself as listeners to events. Furthermore, it is good practice to enclose all of your code in a function, to avoid polluting the global JavaScript scope.

I suggest making a <label> element to go with every <input> field in your HTML. That way, if you click anywhere on the label, the input field will be focused.

function fontDemo() {
  var displayFields = document.querySelectorAll(".fontDisplay");
  var inputField = document.querySelector("#inputField");

  displayFields.forEach(function(f) {
    f.style.fontFamily = f.getAttribute('title');
    f.innerText = '\n';      // Placeholder for spacing
  });
  inputField.addEventListener('input', showWhatITyped);

  function showWhatITyped() {
    displayFields.forEach(function(f) {
      f.innerText = inputField.value || '\n';
    });
  }
}

document.addEventListener('DOMContentLoaded', fontDemo);
.fontDisplay {
  margin-top: 0.5em;
  font-size: 20px;
}

.fontDisplay::before { /* Make the title attribute visible */
  content: attr(title);
  display: block;
  font: medium serif;
}
<html>
<head>
	<title>JavaScript Course</title>
	<script type="text/javascript" src="script.js"></script>
	<link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
	<p><label>Type text here: <input type="text" id="inputField"></label></p>
	<p>What you typed:</p>
	<div class="fontDisplay" title="Times New Roman"></div>
	<div class="fontDisplay" title="Arial"></div>
	<div class="fontDisplay" title="Courier"></div>
	<div class="fontDisplay" title="Verdana"></div>
</body>
</html>

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ You can use data-* attributes to add other things like color and size. <div class="fontDisplay" data-color="red" data-size="12px" data-font="Times New Roman"></div>. Example \$\endgroup\$ – Kruga Mar 6 '18 at 13:39
1
\$\begingroup\$

JavaScript

var field, theDivTNR, theDivA;

Why are these in a global scope? It's a good habit to always create your own scope so that you don't have unwanted clashes between scripts which use the same names.

  console.log("page loaded and DOM is ready");

This is debug code, and should be deleted before the code is ready for review.

// The next function is called each type a key has been
// typed in the input field
function showWhatITyped() {
  // fill the div with the content of the input field
  theDivTNR.innerHTML = field.value;
  theDivA.innerHTML = field.value;
}

What if field.value is not valid HTML? I think this should set textContent instead of innerHTML.

HTML

  <script type="text/javascript" src="script.js"></script>

If you have src you don't need type, because the type can be determined from the HTTP headers.

<body onload="init();">

This is quite ugly and very old-fashioned. See e.g. https://stackoverflow.com/q/799981/573420

  <p>Type text here: <input type="text" oninput= "showWhatITyped();" id="inputField" ></p>

That oninput is even more ugly, and wholly unnecessary. Just refactor the init method to say

  document.querySelector("#inputField").oninput = showWhatITyped;

You may notice that this eliminates field. It's not necessary to store it, provided that you refactor the listener to handle the event which is passed to it. And, in fact, you can also refactor it to be an anonymous function.

  <p>TimesNewRoman</p>
  <div id="theDivTNR"></div>
  <p>Arial</p>
  <div id="theDivA"></div>

You asked how to expand to more fonts etc. The simple answer is to use a loop rather than implementing it by hand. Add class="preview" to each of the preview divs, and then use document.querySelectorAll('.preview') to get hold of them.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Normally, you would indeed separate style into CSS, logic into JS and content in HTML. But your case is quite an unusual case that I think justifies setting the styles from javascript.

JavaScript portion:

  • Create an array
  • Each object in the array represents one presentation (font, color, etc)
  • Loop through the array. For each object in the array, add a div element to the page with the specified formatting. Let all of them have at least one shared class.
  • To show text from user, find all elements with the shared class ans set the text of each of them with the user text.

CSS portion:

  • Only stuff shared by all elements. Don't have the font/color specific stuff here.

HTML portion:

  • Don't have the font/color specific stuff here either.
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.