Haha, you can’t reply anymore because of the 5-reply limit!
blows raspberry 
3 Likes
Thank you Minion.
Improving the code that shows different options is the focus of this post.
Currently we have the following code for showing the options:
var widthOption = widthSelect.options[widthSelect.selectedIndex];
var widthChoice = widthOption.text;
var options = alignments.querySelectorAll(".dd-option");
if (widthChoice === "100%") {
$(alignments).ddslick('select', {index: 1});
options[2].classList.remove("hide");
}
if (widthChoice === "50%") {
$(alignments).ddslick('select', {index: 3});
options[4].classList.remove("hide");
options[5].classList.remove("hide");
}
if (widthChoice === "33%") {
$(alignments).ddslick('select', {index: 6});
options[7].classList.remove("hide");
options[8].classList.remove("hide");
options[9].classList.remove("hide");
}
if (widthChoice === "25%") {
$(alignments).ddslick('select', {index: 10});
options[11].classList.remove("hide");
options[12].classList.remove("hide");
options[13].classList.remove("hide");
options[14].classList.remove("hide");
}
While it works, it is brittle and breaks the with the slightest change to the options list, as we saw earlier.
Moving if statement code to a separate function
What we could do with is a function that takes a starting option index for the option header, and does what’s needed there instead. That way the if statements will be greatly simplified.
if (widthChoice === "100%") {
showOptions(options, 1);
}
if (widthChoice === "50%") {
showOptions(options, 3);
}
if (widthChoice === "33%") {
showOptions(options, 6);
}
if (widthChoice === "25%") {
showOptions(options, 10);
}
We just need that showOptions function to select the given option for the option heading, and to loop through ones after that showing each one, until it gets to one with no value attribute.
Let’s make a start on that, using the 25% options:
showOptions(options, optionIndex) {
$(alignments).ddslick('select', {index: optionIndex});
optionIndex += 1;
options[optionIndex].classList.remove("hide");
optionIndex += 1;
options[optionIndex].classList.remove("hide");
optionIndex += 1;
options[optionIndex].classList.remove("hide");
optionIndex += 1;
options[optionIndex].classList.remove("hide");
}
...
if (widthChoice === "25%") {
showOptions(options, 10);
}
That’s improved the if statement, and resulted in a the problem being smaller, contained to just the showOptions function.
Having showOptions know when to end
Because a different number of options need to be shown, we could pass another function parameter for how many to show, but a more reliable way would be to check if the next option has no value, and stop there instead.
I deliberately set up the showOptions function so that it can be easily turned in to a while loop.
function showOptions(options, optionIndex) {
$(alignments).ddslick('select', {index: optionIndex});
optionIndex += 1;
while (optionIndex < 15) {
options[optionIndex].classList.remove("hide");
optionIndex += 1;
}
}
And now the while loop just needs to know if an option has an empty value or not. We’ll need to check for two things. First, we might have indexed beyond the last option, so we should check if an option exists, and then check if ddSlick has given it a dd-option-value section.
while (
options[optionIndex] &&
options[optionIndex].querySelector(".dd-option-value")
) {
options[optionIndex].classList.remove("hide");
optionIndex += 1;
}
That while condition is looking a bit complex though, so I’ll move it in to a separate function.
function hasOptionValue(ddSlickOption) {
if (!ddSlickOption) {
return;
}
return Boolean(ddSlickOption.querySelector(".dd-option-value"));
}
...
while (hasOptionValue(options[optionIndex])) {
options[optionIndex].classList.remove("hide");
optionIndex += 1;
}
And that seems to be ready to use with the other if statements.
var widthChoice = widthOption.text;
var options = alignments.querySelectorAll(".dd-option");
if (widthChoice === "100%") {
showOptions(options, 1);
}
if (widthChoice === "50%") {
showOptions(options, 3);
}
if (widthChoice === "33%") {
showOptions(options, 6);
}
if (widthChoice === "25%") {
showOptions(options, 10);
}
Haha, that works - neat. https://jsfiddle.net/pmw57/8ycv79qs/2/
Now instead of hard-coding the option heading indexes, we just need to find those option headings and pass their indexes instead, which I’ll work on next.
2 Likes
Here’s the code that we’re looking to improve in this post:
var widthOption = widthSelect.options[widthSelect.selectedIndex];
var widthChoice = widthOption.text;
var options = alignments.querySelectorAll(".dd-option");
if (widthChoice === "100%") {
showOptions(options, 1);
}
if (widthChoice === "50%") {
showOptions(options, 3);
}
if (widthChoice === "33%") {
showOptions(options, 6);
}
if (widthChoice === "25%") {
showOptions(options, 10);
}
Those widthChoice also exist in the option headings, so we should be able to filter through the options for matching option headings, and return the appropriate value.
Making a slow but reliable approach
Instead of doing all of the filtering and matching and returning index values at the same time, we can do this in a much more reliable manner by doing a piece at a time.
First we can assign a headingIndex in each if statement, and move the showOptions call to after the if statements.
Then, we can move those if statements to a separate function.
Then we can replace those if statement with some slightly smarter code.
Assign a headingIndex
The simplest thing to do first is to remove the duplication with the showOptions calls, by moving them out of the if statements to a separate call afterwards.
var ddSlickOptions = alignments.querySelectorAll(".dd-option");
var headingIndex;
if (widthChoice === "100%") {
headingIndex = 1;
}
if (widthChoice === "50%") {
headingIndex = 3;
}
if (widthChoice === "33%") {
headingIndex = 6;
}
if (widthChoice === "25%") {
headingIndex = 10;
}
showOptions(ddSlickOptions, headingIndex);
Move if statements to a separate function
We can now assign headingIndex from a function call, moving those if statements into the function:
function getHeadingIndex(widthChoice, ddSlickOptions) {
var headingIndex;
if (widthChoice === "100%") {
headingIndex = 1;
}
if (widthChoice === "50%") {
headingIndex = 3;
}
if (widthChoice === "33%") {
headingIndex = 6;
}
if (widthChoice === "25%") {
headingIndex = 10;
}
return headingIndex;
}
...
// var headingIndex;
var headingIndex = getHeadingIndex(widthChoice, ddSlickOptions);
showOptions(ddSlickOptions, headingIndex);
And now that the problem code is in a separate function, it’s easier to work on things from there.
Improve the getHeadingIndex function
I know that we’ll want a loop in the getHeadingIndex function, but I’m not sure about the details of it yet. Here’s what I’m starting with:
ddSlickOptions.forEach(function (ddSlickOption, index) {
var label = ddSlickOption.querySelector(".dd-option-text");
console.log(label);
});
The label text I’ll want to match up with the widthChoice, but there are many different ways to get the label text. There’s innerText, outerText, textContent, and innerHTML that all give the text.
Fortunately people have already written about this, for example: What’s Best: innerText vs. innerHTML vs. textContent and based on that, I’ll use innerText that ignores formatting issues.
ddSlickOptions.forEach(function (ddSlickOption, index) {
var label = ddSlickOption.querySelector(".dd-option-text");
var text = label.innerText;
if (text.includes(widthChoice)) {
headingIndex = index;
}
});
And we can now replace the if statements in the getHeadingIndex function:
function getHeadingIndex(widthChoice, ddSlickOptions) {
var headingIndex;
ddSlickOptions.forEach(function (ddSlickOption, index) {
var label = ddSlickOption.querySelector(".dd-option-text");
var text = label.innerText;
if (text.includes(widthChoice)) {
headingIndex = index;
}
});
return headingIndex;
}
We do though have an empty variable assignment at the start, with a return at the end, which can be improved.
Improving the forEach loop
Instead of looping through each one, I want to create a separate array of both the option and its index value, so that I can find the appropriate option and return the matching index value.
So first I’ll use map, to change the options to an array of text values and indexes. The querySelector arrays don’t support fancier techniques like map, so I’ll have to convert it to a proper array before working with it from there.
function getHeadingIndex(widthChoice, ddSlickOptions) {
var headingIndex;
Array.from(ddSlickOptions).map(function (ddSlickOption, index) {
var label = ddSlickOption.querySelector(".dd-option-text");
var text = label.innerText;
return {text, index};
}).forEach(function ({text, index}) {
if (text.includes(widthChoice)) {
headingIndex = index;
}
});
return headingIndex;
}
And the forEach can now be replaced with a find statement instead:
function getHeadingIndex(widthChoice, ddSlickOptions) {
return Array.from(ddSlickOptions).map(function (ddSlickOption, index) {
var label = ddSlickOption.querySelector(".dd-option-text");
var text = label.innerText;
return {text, index};
}).find(function ({text, index}) {
return text.includes(widthChoice);
})["index"];
}
Simplifying the code
That map function is going in to a lot more detail than needs to be there, so let’s move the code out to a separate function called headingIndexes:
function headingIndexes(ddSlickOptions) {
return Array.from(ddSlickOptions).map(function (ddSlickOption, index) {
var label = ddSlickOption.querySelector(".dd-option-text");
var text = label.innerText;
return {text, index};
});
}
function getHeadingIndex(widthChoice, ddSlickOptions) {
return headingIndexes(ddSlickOptions).find(
function ({text, index}) {
return text.includes(widthChoice);
})["index"];
}
And that completes the work.
The final code
The full and final code uses option headings to let it work well when there is no JavaScript, and are also used by the scripting to easily find each appropriate section:
<select id="width">
<option></option>
<option>100%</option>
<option>50%</option>
<option>33%</option>
<option>25%</option>
</select>
<select id="alignment">
<option value="" disabled>Please select a Width</option>
<option value="" disabled>100% Options</option>
<option value="0" data-imagesrc="images/Alignment_0.png">None</option>
<option value="" disabled>50% Options</option>
<option value="1" data-imagesrc="images/Alignment_1.png">Left</option>
<option value="2" data-imagesrc="images/Alignment_2.png">Right</option>
<option value="" disabled>33% Options</option>
<option value="3" data-imagesrc="images/Alignment_3.png">Left</option>
<option value="4" data-imagesrc="images/Alignment_4.png">Center</option>
<option value="5" data-imagesrc="images/Alignment_5.png">Right</option>
<option value="" disabled>25% Options</option>
<option value="6" data-imagesrc="images/Alignment_6.png">Left</option>
<option value="7" data-imagesrc="images/Alignment_7.png">Center-Left</option>
<option value="8" data-imagesrc="images/Alignment_8.png">Center-Right</option>
<option value="9" data-imagesrc="images/Alignment_9.png">Right</option>
</select>
The full scripting code is:
$('#alignment').ddslick({
selectText: "Select your alignment"
});
(function(d) {
"use strict";
var widths = d.querySelector("#width");
var alignments = d.querySelector("#alignment");
function hideDDSlickOptions(select) {
var options = alignments.querySelectorAll(".dd-option");
options.forEach(function (option) {
option.classList.add("hide");
});
}
function hasOptionValue(ddSlickOption) {
if (!ddSlickOption) {
return;
}
return Boolean(ddSlickOption.querySelector(".dd-option-value"));
}
function showOptions(options, optionIndex) {
$(alignments).ddslick('select', {index: optionIndex});
optionIndex += 1;
while (hasOptionValue(options[optionIndex])) {
options[optionIndex].classList.remove("hide");
optionIndex += 1;
}
}
function headingIndexes(ddSlickOptions) {
return Array.from(ddSlickOptions).map(function (ddSlickOption, index) {
var label = ddSlickOption.querySelector(".dd-option-text");
var text = label.innerText;
return {text, index};
});
}
function getHeadingIndex(widthChoice, ddSlickOptions) {
return headingIndexes(ddSlickOptions).find(
function ({text, index}) {
return text.includes(widthChoice);
})["index"];
}
function showAlignmentOptions(widthSelect, alignments) {
hideDDSlickOptions(alignments);
var widthOption = widthSelect.options[widthSelect.selectedIndex];
var widthChoice = widthOption.text;
var ddSlickOptions = alignments.querySelectorAll(".dd-option");
var headingIndex = getHeadingIndex(widthChoice, ddSlickOptions);
showOptions(ddSlickOptions, headingIndex);
}
function widthChangeHandler(evt) {
var widthChoice = evt.target;
showAlignmentOptions(widthChoice, alignments);
}
widths.addEventListener("change", widthChangeHandler);
hideDDSlickOptions(alignments);
$(alignments).ddslick('select', {index: 0});
}(document));
and a working example can be played with at https://jsfiddle.net/pmw57/8ycv79qs/3/
2 Likes
I should just finish up by saying that this code would have been much simpler without ddSlick being involved. Normally it’s as simple as this:
options.forEach(function (option) {
var type = option.dataset["type"];
var shouldHide = !type || !type.includes(widthChoice);
option.classList.toggle("hide", shouldHide);
});
And that’s it. Here’s some sample code that uses that really simple way of doing things. https://jsfiddle.net/pmw57/8ycv79qs/5/
Even though ddSlick does makes things more difficult, we’ve used some well-named functions to help keep things relatively simple, while still working within the constrants given to us.
2 Likes
This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.