Skip to content

Commit

Permalink
fix: simplify logic for hiding/showing all columns within show/hide menu
Browse files Browse the repository at this point in the history
The functionality to show/hide all columns within the ShowHideColumnsMenu was implemented with a custom function which filters all columns and toggles the visibility of each column. This calls the `onColumnVisibilityChange` callback multiple times.

In my opinion this can be refactored to use the tables `toggleAllColumnsVisible` API, which results in only one call of `onColumnVisibilityChange` and makes the code easier to understand.

Fixes: #1277
  • Loading branch information
fredericbahr authored Dec 21, 2024
1 parent 475f98d commit 03988f6
Showing 1 changed file with 2 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export const MRT_ShowHideColumnsMenu = <TData extends MRT_RowData>({
}: MRT_ShowHideColumnsMenuProps<TData>) => {
const {
getAllColumns,
getAllLeafColumns,
getCenterLeafColumns,
getIsAllColumnsVisible,
getIsSomeColumnsPinned,
Expand All @@ -45,12 +44,6 @@ export const MRT_ShowHideColumnsMenu = <TData extends MRT_RowData>({
} = table;
const { columnOrder, columnPinning, density } = getState();

const handleToggleAllColumns = (value?: boolean) => {
getAllLeafColumns()
.filter((col) => col.columnDef.enableHiding !== false)
.forEach((col) => col.toggleVisibility(value));
};

const allColumns = useMemo(() => {
const columns = getAllColumns();
if (
Expand Down Expand Up @@ -108,7 +101,7 @@ export const MRT_ShowHideColumnsMenu = <TData extends MRT_RowData>({
{enableHiding && (
<Button
disabled={!getIsSomeColumnsVisible()}
onClick={() => handleToggleAllColumns(false)}
onClick={() => table.toggleAllColumnsVisible(false)}
>
{localization.hideAll}
</Button>
Expand All @@ -135,7 +128,7 @@ export const MRT_ShowHideColumnsMenu = <TData extends MRT_RowData>({
{enableHiding && (
<Button
disabled={getIsAllColumnsVisible()}
onClick={() => handleToggleAllColumns(true)}
onClick={() => table.toggleAllColumnsVisible(true)}
>
{localization.showAll}
</Button>
Expand Down

0 comments on commit 03988f6

Please sign in to comment.