Skip to content

Commit

Permalink
Show/hide columns using display instead of visibility
Browse files Browse the repository at this point in the history
Visibility was way simpler, but the table doesn't recalculate column widths until you use display:none;

This is now using the same method as the old products screen.
But we still need to update colspans..
  • Loading branch information
dacook committed Jun 13, 2024
1 parent 0190d6f commit 70fab2b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
25 changes: 22 additions & 3 deletions app/webpacker/controllers/column_preferences_controller.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { Controller } from "stimulus";


// Manage column visibility according to checkbox selection
//
export default class ColumnPreferencesController extends Controller {
connect() {
this.checkboxes = this.element.querySelectorAll('input[type=checkbox]');
this.checkboxes = this.element.querySelectorAll("input[type=checkbox]");

for (const element of this.checkboxes) {
// On initial load
Expand All @@ -22,12 +21,32 @@ export default class ColumnPreferencesController extends Controller {
const name = element.dataset.columnName;
const selector = `col[data-column-preferences-name="${name}"]`;
const column = document.querySelector(selector);
const index = this.#getIndex(column);

if (column == null) {
console.error(`ColumnPreferencesController: could not find ${selector}`);
return;
}

column.style.visibility = (element.checked ? '' : 'collapse');
// Hide column definition
this.#showHideElement(column, element.checked);

// Hide each cell in column (ignore rows with colspan)
const rows = column.closest("table").querySelectorAll("tr:not(:has(td[colspan]))");
rows.forEach((row) => {
// Ignore cell if spanning multiple columns
const cell = row.children[index];
if (cell == undefined) return;

this.#showHideElement(cell, element.checked);
});
}

#getIndex(column) {
return Array.from(column.parentNode.children).indexOf(column);
}

#showHideElement(element, show) {
element.style.display = show ? "" : "none";
}
}
4 changes: 2 additions & 2 deletions spec/system/admin/products_v3/products_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
within ofn_drop_down("Columns") do
uncheck "Name"
end
# expect(page).not_to have_selector "th", text: "Name" # column is not visible, but capybara doesn't understand yet.
expect(page).not_to have_selector "th", text: "Name"
expect_other_columns_visible

# Preference saved
Expand All @@ -62,7 +62,7 @@
within ofn_drop_down("Columns") do
expect(page).to have_unchecked_field "Name"
end
# expect(page).not_to have_selector "th", text: "Name"
expect(page).not_to have_selector "th", text: "Name"
expect_other_columns_visible
end

Expand Down

0 comments on commit 70fab2b

Please sign in to comment.