Do I have bad logic for changing innerHTML on click?

JavaScript
1

I have a virtual keyboard where clicking SHIFT adds a class to highlight that the SHIFT key is on. I using that class to change the innerHTML on some of the double character keys like right square bracket and right curly bracket: [ and {.

That works and then clicking that virtual key enters [ or { into the textarea. I’ll have to do that for a total of 8 keys, for \ ; ’ , . and /. Those keys and the characters above them are what I want to change when you click SHIFT.

I’m wondering if I am repeating myself and if there is an easier way. Here is what I have for the right and left square bracket keys:

const shiftKeyL = document.querySelector(".shiftL");
const shiftKeyR = document.querySelector(".shiftR");
let rightSquare = document.getElementById("rtsq");
let leftSquare = document.getElementById("leftsq");

function changeInner() {
  if (shiftl.classList.contains('shift-on') || shiftr.classList.contains('shift-on')) {
    rightSquare.innerHTML = "{";
    leftSquare.innerHTML = "}";
  } else {
    rightSquare.innerHTML = "[";
    leftSquare.innerHTML = "]";
  }
};
shiftKeyL.addEventListener("click", changeInner);
shiftKeyR.addEventListener("click", changeInner);

Is there a better way, or is it okay to have 6 more lines in my if and else blocks?

2

there is not a limit on how many lines can go in there

Does it behave how you want? that’s important

3

I just thought that I might be writing sloppy and unnecessary code. It all works, except I cannot seem to output less than and greater than signs. Instead, when I click on those buttons I get the HTML entity for each in my textarea. My only other “fail” is not figuring out how to get the up and down arrow keys to work. I might have to look into clientX and clientY a little more deeply.

4

If there is no other reason in your code to need to know which shift key was pressed, right or left. You could put a class name of just “shiftKey” on both shift keys.

Then one line:
const shiftKeys = document.querySelectorAll(".shiftKey")

Add the listener, one line:
shiftKeys.forEach( x => x.addEventListener("click", changeInner)

Then in your function remove the || in if and utilize the event passed to the function:

function changeInner(event) {
    if (event.target.classList.contains('shift-on') {
     // change stuff 
    }
}

That eliminates some semi-repeated code. You likely would have to modify other code accordingly for how you add and remove the class of shift-on but that might be a place that some code can be shortened also.

clientX and clientY “Output the coordinates of the mouse pointer when the mouse button is clicked on an element”

So, it seems those might not be the way to go since I would assume you just want to know if the up or down arrow is pressed, and it wouldn’t matter where the arrow elements were on the screen. But without seeing the code that is just my first thought.

5

I think I tried a class of just “shift” but it only worked for the first DOM element with that class, the left shift key. My right shift would light up but it wouldn’t display the shift-keys alt values. I’ll have to look at it later.

As for the up keys, I was thinking I need to know where the active cursor is in the textarea, so that I could keep the X value the same and change the Y value to move the cursor up the height of a line. That’s a wish-list item and I’ll have to figure that one out later as well. Thanks!

6

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.