Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions templates/js/dropdowns.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,21 @@ boneSelect.addEventListener("change", (e) => {
loadDescription(selectedBoneId);

const opts = { boneId: selectedBoneId };

loadBoneImages(selectedBoneId, opts);
} else {
const stage = getImageStage();
if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); }
showPlaceholder();

// Deselecting the bone shouldn't blank the display if a boneset is still
// selected (Issue #248) - fall back to showing that boneset's info.
const selectedBonesetId = bonesetSelect.value;
if (selectedBonesetId) {
loadDescription(selectedBonesetId);
loadBoneImages(selectedBonesetId, { boneId: selectedBonesetId });
} else {
showPlaceholder();
}
}
});

Expand All @@ -184,7 +193,15 @@ subboneSelect.addEventListener("change", (e) => {

loadBoneImages(selectedSubboneId, opts);
} else {
showPlaceholder();
// Same fallback as the bone-change listener above, one level down:
// deselecting the sub-bone should reveal the parent bone's info, not go blank.
const selectedBoneId = boneSelect.value;
if (selectedBoneId) {
loadDescription(selectedBoneId);
loadBoneImages(selectedBoneId, { boneId: selectedBoneId });
} else {
showPlaceholder();
}
}
});

Expand Down
3 changes: 1 addition & 2 deletions templates/js/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { fetchCombinedData, fetchBoneData } from "./api.js";
import { populateBonesetDropdown, setupDropdownListeners } from "./dropdowns.js";
import {initializeSidebar} from "./sidebar.js";
import { setupNavigation, setBoneAndSubbones, disableButtons } from "./navigation.js";
import { loadDescription } from "./description.js";
import { displayBoneData, clearViewer } from "./viewer.js";
import { initializeSearch } from "./search.js";
import quizManager from "./quiz.js";
Expand Down Expand Up @@ -47,7 +46,7 @@ document.addEventListener("DOMContentLoaded", async () => {
// Log fetched data for troubleshooting when dropdowns are empty
console.debug("combinedData:", combinedData);

setupNavigation(prevButton, nextButton, subboneDropdown, loadDescription);
setupNavigation(prevButton, nextButton, subboneDropdown);
// Initialize button states
disableButtons(prevButton, nextButton);

Expand Down
36 changes: 20 additions & 16 deletions templates/js/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,16 @@ let subbones = [];
* @param {HTMLButtonElement} prevButton - The "previous" navigation button.
* @param {HTMLButtonElement} nextButton - The "next" navigation button.
* @param {HTMLSelectElement} subboneDropdown - The subbone `<select>` element to keep in sync.
* @param {function(string): void} updateDescription - Callback invoked with the selected subbone ID
* whenever the navigation changes.
* @returns {void}
*/
export function setupNavigation(prevButton, nextButton, subboneDropdown, updateDescription) {
export function setupNavigation(prevButton, nextButton, subboneDropdown) {
// Setup Previous/Next button navigation
prevButton.addEventListener("click", () => {
prevSubbone();
updateUI(subboneDropdown, updateDescription);
if (prevSubbone()) updateUI(subboneDropdown);
});

nextButton.addEventListener("click", () => {
nextSubbone();
updateUI(subboneDropdown, updateDescription);
if (nextSubbone()) updateUI(subboneDropdown);
});

disableButtons(prevButton, nextButton);
Expand Down Expand Up @@ -83,37 +79,45 @@ export function setBoneAndSubbones(bone, boneSubbones) {

/**
* Decrements the current subbone index (moves to the previous subbone), if greater than 0.
* @returns {void}
* @returns {boolean} True if the index moved, false if already at the first subbone.
*/
function prevSubbone() {
if (currentSubboneIndex > 0) {
currentSubboneIndex--;
return true;
}
return false;
}

/**
* Increments the current subbone index (moves to the next subbone), if less than the array of subbones.
* @returns {void}
* @returns {boolean} True if the index moved, false if already at the last subbone.
*/
function nextSubbone() {
if (currentSubboneIndex < subbones.length - 1) {
currentSubboneIndex++;
return true;
}
return false;
}

/**
* Syncs the subbone dropdown to the current index and invokes the description callback.
* Does nothing if no subbones are loaded.
* Syncs the subbone dropdown to the current index and dispatches a native "change"
* event so the same listeners that handle a manual dropdown selection (description,
* image, and annotation loading) also run for Prev/Next navigation. Setting
* `selectedIndex` alone does not fire "change", which is why those listeners were
* previously skipped. The dropdown is set by `.value` rather than `.selectedIndex`
* because the real `<select>` has a placeholder option before the subbone options,
* so `currentSubboneIndex` (0-based into `subbones`) does not match the option's
* position in the dropdown. Does nothing if no subbones are loaded.
* @param {HTMLSelectElement} subboneDropdown - The subbone select element to update.
* @param {function(string): void} updateDescription - Callback invoked with the selected subbone ID.
* @returns {void}
*/
function updateUI(subboneDropdown, updateDescription) {
function updateUI(subboneDropdown) {
if (subbones.length === 0 || currentSubboneIndex === -1) return;

subboneDropdown.selectedIndex = currentSubboneIndex;
const selectedSubbone = subbones[currentSubboneIndex];
updateDescription(selectedSubbone);
subboneDropdown.value = subbones[currentSubboneIndex];
subboneDropdown.dispatchEvent(new Event("change"));
}

/**
Expand Down
89 changes: 89 additions & 0 deletions templates/tests/dropdowns.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
jest.mock("../js/description.js", () => ({
loadDescription: jest.fn(),
}));
jest.mock("../js/imageDisplay.js", () => ({
displayBoneImages: jest.fn(),
showPlaceholder: jest.fn(),
}));
jest.mock("../js/annotationOverlay.js", () => ({
clearAnnotations: jest.fn(),
}));
jest.mock("../js/api.js", () => ({
fetchBoneData: jest.fn(() => Promise.resolve({ images: [{ url: "test.jpg" }] })),
}));

const { loadDescription } = require("../js/description.js");
const { showPlaceholder } = require("../js/imageDisplay.js");
const { setupDropdownListeners, populateBonesetDropdown } = require("../js/dropdowns.js");

const combinedData = {
bonesets: [{ id: "bony_pelvis", name: "Bony Pelvis" }],
bones: [{ id: "ilium", name: "Ilium", boneset: "bony_pelvis" }],
subbones: [{ id: "iliac_crest", name: "Iliac Crest", bone: "ilium" }],
};

function renderDropdownsHTML() {
document.body.innerHTML = `
<select id="boneset-select"></select>
<select id="bone-select"></select>
<select id="subbone-select"></select>
<div id="bone-image-container"></div>
`;
}

function selectValue(select, value) {
select.value = value;
select.dispatchEvent(new Event("change"));
}

describe("Deselecting a bone/sub-bone reverts to parent info - Issue 248", () => {
let bonesetSelect, boneSelect, subboneSelect;

beforeEach(() => {
jest.clearAllMocks();
renderDropdownsHTML();

bonesetSelect = document.getElementById("boneset-select");
boneSelect = document.getElementById("bone-select");
subboneSelect = document.getElementById("subbone-select");

// Populate the boneset options; the bone/sub-bone options are populated
// dynamically by the listeners themselves as each level is selected.
populateBonesetDropdown(combinedData.bonesets);
setupDropdownListeners(combinedData);
});

test("deselecting a bone falls back to the boneset info instead of the placeholder", async () => {
selectValue(bonesetSelect, "bony_pelvis");
selectValue(boneSelect, "ilium");
loadDescription.mockClear();
showPlaceholder.mockClear();

selectValue(boneSelect, "");
await new Promise(process.nextTick);

expect(loadDescription).toHaveBeenCalledWith("bony_pelvis");
expect(showPlaceholder).not.toHaveBeenCalled();
});

test("deselecting a sub-bone falls back to the parent bone info instead of the placeholder", async () => {
selectValue(bonesetSelect, "bony_pelvis");
selectValue(boneSelect, "ilium");
selectValue(subboneSelect, "iliac_crest");
loadDescription.mockClear();
showPlaceholder.mockClear();

selectValue(subboneSelect, "");
await new Promise(process.nextTick);

expect(loadDescription).toHaveBeenCalledWith("ilium");
expect(showPlaceholder).not.toHaveBeenCalled();
});

test("deselecting a bone with no boneset selected still shows the placeholder", async () => {
selectValue(boneSelect, "");
await new Promise(process.nextTick);

expect(showPlaceholder).toHaveBeenCalled();
});
});
83 changes: 83 additions & 0 deletions templates/tests/navigation.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
const {
setupNavigation,
setBoneAndSubbones,
disableButtons,
} = require("../js/navigation.js");

function renderNavigationHTML() {
document.body.innerHTML = `
<button id="prev-button"></button>
<button id="next-button"></button>
<span id="text-button-Home" role="button"></span>
<select id="subbone-select">
<option value="">--Please choose a Bone Part--</option>
<option value="subbone_a">Subbone A</option>
<option value="subbone_b">Subbone B</option>
<option value="subbone_c">Subbone C</option>
</select>
`;
}

describe("Prev/Next navigation keeps the dropdown and its listeners in sync - Issue 249", () => {
let prevButton, nextButton, subboneDropdown, changeSpy;

beforeEach(() => {
renderNavigationHTML();
prevButton = document.getElementById("prev-button");
nextButton = document.getElementById("next-button");
subboneDropdown = document.getElementById("subbone-select");

setupNavigation(prevButton, nextButton, subboneDropdown);
setBoneAndSubbones("ilium", ["subbone_a", "subbone_b", "subbone_c"]);
disableButtons(prevButton, nextButton);

// Standing in for the real listeners in dropdowns.js / HTMX that only
// run when a real "change" event fires on the subbone dropdown.
changeSpy = jest.fn();
subboneDropdown.addEventListener("change", changeSpy);
});

test("clicking Next dispatches a change event and advances the dropdown selection", () => {
nextButton.click();

expect(subboneDropdown.value).toBe("subbone_b");
expect(changeSpy).toHaveBeenCalledTimes(1);
});

test("clicking Next repeatedly stops at the last subbone", () => {
nextButton.click(); // subbone_a -> subbone_b
nextButton.click(); // subbone_b -> subbone_c
nextButton.click(); // already last subbone: no-op, no extra change event

expect(subboneDropdown.value).toBe("subbone_c");
expect(changeSpy).toHaveBeenCalledTimes(2);
});

test("clicking Previous dispatches a change event and moves back a subbone", () => {
nextButton.click();
nextButton.click();
changeSpy.mockClear();

prevButton.click();

expect(subboneDropdown.value).toBe("subbone_b");
expect(changeSpy).toHaveBeenCalledTimes(1);
});

test("clicking Previous at the first subbone does nothing", () => {
const valueBeforeClick = subboneDropdown.value;

prevButton.click();

expect(subboneDropdown.value).toBe(valueBeforeClick);
expect(changeSpy).not.toHaveBeenCalled();
});

test("buttons are disabled when the current bone has no subbones", () => {
setBoneAndSubbones("ischium", []);
disableButtons(prevButton, nextButton);

expect(prevButton.disabled).toBe(true);
expect(nextButton.disabled).toBe(true);
});
});
Loading