I am a hobbyist/novice.
This is very difficult to describe, I can't seem to wrap my head around what is actually happening and that is making the problem difficult to debug.
I have my script divided into three objects and a class, each object holds Elements of an HTML page, Event Listeners, and Functions. I will ask about the merits and flaws of this organization in a separate question.
This project is meant to provide an easy reference for the DM of a tabletop RPG called "Apocalypse World" which is pretty basic. On this webpage, I perform a few different actions but the one in question is the basic moves reference. I have a drop-down list that has the name of the moves, you pick one, and it displays a short summary of the move. This seems to work fine in my first iteration but I wanted to add a feature where selecting the move that is already being displayed will make that move toggle a hidden class that will remove the summary from view. Displaying the first summary seems to work and hiding the element seems to work, but when you try to display a different element both the selected element and the previous element are displayed and have to be removed individually.
Truncated Code
- The complete code can be found here.
- The offending code is located in script.js:
Functions.basicMoves.displayBasicMove
script.js
// HTML Elements
const Elements = {
... ,
basicMoves: {
selector: document.querySelector("#basicMovesSelector"),
button: document.querySelector("#getMove"),
// 1. The currentElement holds the element that was last revealed.
currentElement: null
}
};
// Event Listeners
const Listeners = {
... ,
basicMoves: {
l1: Elements.basicMoves.button.addEventListener("click", (e) => { Functions.displayBasicMove(e) })
}
};
// Functions
const Functions = {
... ,
displayBasicMove: (e) => {
e.preventDefault();
// 2. The next element to be revealed or the current element is selected
const newElement = document.querySelector(`#${Elements.basicMoves.selector.value}`);
// This is the offending if-else block.
if (Elements.basicMoves.currentElement === null) {
// 3. If currentElement is null, make the new element the current element, then toggle visibility on that element.
Elements.basicMoves.currentElement = newElement;
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
else if (Elements.basicMoves.currentElement === newElement) {
// 4. If the same element is selected, toggle the hidden class on that element.
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
else {
// 5. If currentElement is NOT null AND NOT the same element, toggle the hidden class on the current element, set the new element as the current element, then toggle that element.
Elements.basicMoves.currentElement.classList.toggle("hidden");
Elements.basicMoves.currentElement = newElement;
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
}
};
index.html
<!DOCTYPE html>
<html>
<head>
...
</head>
<body>
<h1>Apocalypse World MC Utilities</h1>
...
<div id="basicMoves">
<form>
<h2>Basic Moves</h2>
<select id="basicMovesSelector">
<option value="under-fire">Act Under Fire</option>
<option value="aggro">Go Aggro</option>
<option value="sucker">Sucker Somebody</option>
<option value="seize">Seize by Force</option>
<option value="seduce-manipulate">Seduce or Manipulate</option>
<option value="read-sitch">Read a Sitch</option>
<option value="read-person">Read a Person</option>
<option value="open-brain">Open Your Brain</option>
<option value="help-interfere">Help or Interfere</option>
<option value="lifestyle">Lifestyle Upkeep</option>
<option value="work-gig">Work a Gig</option>
<option value="session-end">Session End</option>
</select>
<button id="getMove">Show Move</button>
</form>
<div id="basicMovesDescriptors">
<div class="hidden" id="under-fire">
<p>roll+cool</p>
<p>On a 10+, you do it.</p>
<p>On a 7–9, you flinch, hesitate, or stall:
the MC can offer you a worse outcome, a hard bargain, or an ugly
choice.</p>
<p>On a miss, be prepared for the worst.</p>
</div>
<div class="hidden" id="aggro">
<p>roll+hard</p>
<p>On a 10+, they have to choose 1:</p>
<p>• Force your hand and suck it up.</p>
<p>• Cave and do what you want.</p>
<p>On a 7–9, they can choose 1 of the above, or 1 of the following:</p>
<p>• Get the hell out of your way.</p>
<p>• Barricade themselves securely in.</p>
<p>• Give you something they think you want, or tell you what you want to hear.</p>
<p>• Back off calmly, hands where you can see.</p>
<p>On a miss, be prepared for the worst.</p>
</div>
<div class="hidden" id="sucker">
<p>When you attack someone unsuspecting or helpless, ask the
MC if you could miss. If you could, treat it as going aggro, but your
victim has no choice to cave and do what you want. If you couldn’t,
you simply inflict harm as established.</p>
</div>
<div class="hidden" id="seize">
<p>To seize something by force, exchange harm, but first roll+hard.
<p>On a 10+, choose 3. On a 7–9, choose 2. On a miss, choose 1:</p>
<p>• You inflict terrible harm (+1harm).</p>
<p>• You suffer little harm (-1harm).</p>
<p>• You take definite and undeniable control of it.</p>
<p>• You impress, dismay, or frighten your enemy.</p>
</div>
<div class="hidden" id="seduce-manipulate">
<p>roll+hot</p>
<p>For NPCs: on a 10+, they’ll go along with you, unless or
until some fact or action betrays the reason you gave them.</p>
<p>On a 7–9,
they’ll go along with you, but they need some concrete assurance,
corroboration, or evidence first.</p>
<p>For PCs: on a 10+, both.</p>
<p>On a 7–9, choose 1:</p>
<p>• If they go along with you, they mark experience.</p>
<p>• If they refuse, erase one of their stat highlights for the remainder of
the session.</p>
<p>What they do then is up to them.</p>
<p>On a miss, for either NPCs or PCs, be prepared for the worst.</p>
</div>
<div class="hidden" id="read-sitch">
<p>roll+sharp</p>
<p>On a hit, you can ask the MC questions.
Whenever you act on one of the MC’s answers, take +1.</p>
<p>On a 10+, ask 3. On a 7–9, ask 1:</p>
<p>• Where’s my best escape route, way in, or way past?</p>
<p>• Which enemy is most vulnerable to me?</p>
<p>• Which enemy is the biggest threat?</p>
<p>• What should I be on the lookout for?</p>
<p>• What’s my enemy’s true position?</p>
<p>• Who’s in control here?</p>
<p>On a miss, ask 1 anyway, but be prepared for the worst.</p>
</div>
<div class="hidden" id="read-person">
<p>roll+sharp</p>
<p>On a 10+, hold 3.</p>
<p>On a 7–9, hold 1.</p>
<p>While you’re interacting with them,
spend your hold to ask their player questions, 1 for 1:</p>
<p>• Is your character telling the truth?</p>
<p>• What’s your character really feeling?</p>
<p>• What does your character intend to do?</p>
<p>• What does your character wish I’d do?</p>
<p>• How could I get your character to —?</p>
<p>On a miss, ask 1 anyway, but be prepared for the worst.</p>
</div>
<div class="hidden" id="open-brain">
<p>roll+weird</p>
<p>On a hit, the MC tells you something new and interesting
about the current situation, and might ask you a question or two;
answer them.</p>
<p>On a 10+, the MC gives you good detail.</p>
<p>On a 7–9, the
MC gives you an impression. If you already know all there is to know,
the MC will tell you that.</p>
<p>On a miss, be prepared for the worst.</p>
</div>
<div class="hidden" id="help-interfere">
<p>roll+Hx</p>
<p>On a 10+, they take +2 (help) or -2 (interfere) to their roll.</p>
<p>On a 7–9, they take +1 (help) or -1 (interfere) to their roll.</p>
<p>On a miss, be prepared for the worst.</p>
</div>
<div class="hidden" id="lifestyle">
<p>At the beginning of the session, spend 1- or 2-barter for your
lifestyle.</p>
</p>If you can’t or won’t, tell the MC and answer her questions.</p>
<p>1-barter at the beginning of the session, that’s the
quality of life her character’s had and can generally expect. The same as
most people around her.</p>
<p>2-barter at the beginning of the session, she has a quality
of life that is substantially, notably better. Whatever other people eat,
drink, and wear, she’s eating, drinking, and wearing better. She sleeps
more comfortably and safer, and has more control over her personal
environment.</p>
<p>0-barter, this should mean that at the beginning of the session
she’s desperately hungry and dying of thirst, but hold off. It could turn out
that way.</p>
<p>Another PC springs for them;</p>
<p>If none of them can or will, though, you get to choose:</p>
<p>• Go straight to, yes, she’s desperately hungry and dying of thirst. Inflict
harm as established. Take away stuff.</p>
<p>• Choose a suitable NPC; Now the debt is between her and your NPC.</p>
<p>• Ask player which NPC You can either negotiate an appropriate arrangement
in summary, or else jump into play: have her read a person, seduce &
manipulate, go aggro, or whatever she needs to do to get her way.</p>
</div>
<div class="hidden" id="work-gig">
<p>If you need jingle during a session, tell the MC you’d like to work a gig.</p>
</div>
<div class="hidden" id="session-end">
<p>At the end of every session, choose a character who knows you
better than they used to. If there’s more than one, choose one at your
whim. Tell that player to add +1 to their Hx with you on their sheet.
If this brings them to Hx+4, they reset to Hx+1 (and therefore mark
experience).</p>
<p>If no one knows you better, choose a character who
doesn’t know you as well as they thought, or choose any character at
your whim. Tell that player to take -1 to their Hx with you on their
sheet. If this brings them to Hx -3, they reset to Hx=0 (and therefore
mark experience).</p>
</div>
</div>
</div>
...
<script src="script.js"></script>
</body>
</html>
Any help or a nudge in the right direction would be most appreciated.
The Solution
After staring at this for hours and enlisting the help of a friendly coworker, we finally realized what was going wrong.
The Problem
if (Elements.basicMoves.currentElement === null) {
Elements.basicMoves.currentElement = newElement;
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
// #1 This block is the problem!
else if (Elements.basicMoves.currentElement === newElement) {
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
else {
// #2 This is where the problem manifests
Elements.basicMoves.currentElement.classList.toggle("hidden");
Elements.basicMoves.currentElement = newElement;
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
When I toggle a currently displayed element on #1, that element is still stored in Elements.basicMoves.currentElement. When another element is selected #2 goes into effect and toggles the currentElement (causing the previously dismissed element to return), sets the new element to currentElement, then toggles currentElement. The result is that both the dismissed and currently selected elements appear.
The Fix
if (Elements.basicMoves.currentElement === null) {
Elements.basicMoves.currentElement = newElement;
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
else if (Elements.basicMoves.currentElement === newElement) {
Elements.basicMoves.currentElement.classList.toggle("hidden");
// One line of code to fix!
Elements.basicMoves.currentElement = null;
}
else {
Elements.basicMoves.currentElement.classList.toggle("hidden");
Elements.basicMoves.currentElement = newElement;
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
By adding Elements.basicMoves.currentElement = null; to the block that hides the previously selected element, we reset the our state where nothing is displayed, and currentElement is null. This causes the code to function as intended.
The Actual Implementation
While the solution works, my friendly coworker pointed out that having a single button do double duty as a "display" & "clear" button adds unnecessary complexity to the code. So he suggested having two separate buttons which can more clearly indicate their functions and simplify the code.
HTML
<h2>Basic Moves</h2>
<select id="basicMovesSelector">...</select>
<button id="getMove">Show Move</button>
<button id="clearMove">Clear Move</button>
Javascript
//I added a clearButton element.
const Elements = {
... ,
basicMoves: {
selector: document.querySelector("#basicMovesSelector"),
showButton: document.querySelector("#getMove"),
clearButton: document.querySelector("#clearMove"),
currentElement: null
}
};
/* I added a clearButton event listener, and also changed the event
listeners to a Arrays instead of an Object.*/
const Listeners = {
... ,
basicMoves: [
Elements.basicMoves.showButton.addEventListener("click", (e) => { Functions.basicMoves.displayMove(e) }),
Elements.basicMoves.clearButton.addEventListener("click", (e) => { Functions.basicMoves.clearMove(e) })
]
};
const Functions = {
... ,
// I broke up basicMoves into two functions.
basicMoves: {
displayMove: (e) => {
e.preventDefault();
const newElement = document.querySelector(`#${Elements.basicMoves.selector.value}`);
if (Elements.basicMoves.currentElement === null) {
Elements.basicMoves.currentElement = newElement;
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
else {
Elements.basicMoves.currentElement.classList.toggle("hidden");
Elements.basicMoves.currentElement = newElement;
Elements.basicMoves.currentElement.classList.toggle("hidden");
}
},
clearMove: (e) => {
e.preventDefault();
/* He showed me this nifty operater currentElement? that will
check if the value of currentElement is null and skip the rest of the
line if it is instead of having another if-else block. I was going to
use a ternary operator but this is so much cleaner.*/
Elements.basicMoves.currentElement?.classList.toggle("hidden");
Elements.basicMoves.currentElement = null;
}
}
};
Comments
Post a Comment