Skip to content

fix: Resource authorization selection directory checkbox Cancel optimization #4253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shaohuzhang1 merged 1 commit into from
Oct 24, 2025
Merged

fix: Resource authorization selection directory checkbox Cancel optimization #4253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 9 additions & 1 deletion ui/src/components/app-table/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
<el-pagination
v-model:current-page="paginationConfig.current_page"
v-model:page-size="paginationConfig.page_size"
:page-sizes="paginationConfig.page_sizes|| pageSizes"
:page-sizes="paginationConfig.page_sizes || pageSizes"
:total="paginationConfig.total"
layout="total, prev, pager, next, sizes"
@size-change="handleSizeChange"
Expand Down Expand Up @@ -138,8 +138,16 @@ function handleCurrentChange() {
function clearSelection() {
appTableRef.value?.clearSelection()
}
function toggleRowSelection(row: any, selected?: boolean, ignoreSelectable = true) {
appTableRef.value?.toggleRowSelection(row, selected, ignoreSelectable)
}
function getSelectionRows() {
return appTableRef.value?.getSelectionRows()
}
defineExpose({
clearSelection,
toggleRowSelection,
getSelectionRows,
})

onMounted(() => {
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code is generally clean and well-structured. Here are some minor improvements and clarifications:

  1. Remove Unnecessary Comma: There's an unnecessary comma after pageSizes in the first line of the <el-pagination> tag. Remove it to ensure proper syntax.

Optimizations and Suggestions

  1. Avoid Redefinitions: Although not incorrect, there's no need to define a function called handleSizeChange just to call another function (handleCurrentChange). You can directly pass the event handler without defining an additional function.

Here’s the revised version with these adjustments:

<template>
  <!-- Template content here -->
</template>

<script lang="ts">
import { ref, onMounted } from 'vue';
import {
  ElPagination,
} from "element-plus";

export default defineComponent({
  components: {
    ElPagination,
  },
  setup() {
    const paginationConfig = reactive({
      current_page: 1,
      pagesize: 0,
    });

    const appTableRef = ref<Element | undefined>();

    function handleCurrentChange(pageNo) {
      paginationConfig.current_page = pageNo;
      // Add logic to fetch data based on new page number
    }

    function clearSelection() {
      appTableRef.value?.clearSelection();
    }
    
    function toggleRowSelection(row: any, selected?: boolean, ignoreSelectable = true) {
      if (appTableRef.value) {
        appTableRef.value.toggleRowSelection(row, selected, ignoreSelectable);
      }
    }

    function getSelectionRows() {
      return appTableRef.value ? appTableRef.value.getSelectionRows() : [];
    }

    onMounted(() => {
      // Initialization code if needed
    });

    defineExpose({
      clearSelection,
      toggleRowSelection,
      getSelectionRows,
    });
  }
});
</script>

<style scoped>
/* Styles go here */
</style>

These changes make your code slightly more concise and align better with Vue.js conventions.

Expand Down
31 changes: 27 additions & 4 deletions ui/src/views/system/resource-authorization/component/PermissionTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
ref="multipleTableRef"
class="mt-16"
:data="filteredData"
@selection-change="handleSelectionChange"
@select="select"
:maxTableHeight="260"
:row-key="(row: any) => row.id"
:expand-row-keys="defaultExpandKeys"
Expand Down Expand Up @@ -147,6 +147,7 @@ import { hasPermission } from '@/utils/permission/index'
import { ComplexPermission } from '@/utils/permission/type'
import { getPermissionOptions } from '@/views/system/resource-authorization/constant'
import useStore from '@/stores'
import { TreeToFlatten } from '@/utils/array'

const { model, user } = useStore()
const route = useRoute()
Expand Down Expand Up @@ -310,11 +311,33 @@ const filteredData = computed(() => {
})

const multipleSelection = ref<any[]>([])
const selectObj: any = {}
const select = (val: any[], active: any) => {
if (active.resource_type === 'folder') {
if (!val.some((item) => item.id == active.id)) {
if (selectObj[active.id] === undefined) {
selectObj[active.id] = 0
}
if (selectObj[active.id] % 2 == 0) {
console.log(TreeToFlatten([active]))
TreeToFlatten([active])
.filter((item: any) => item.id != active.id)
.forEach((item: any) => {
multipleTableRef.value?.toggleRowSelection(item, true)
})

const handleSelectionChange = (val: any[]) => {
multipleSelection.value = val
multipleSelection.value = multipleTableRef.value.getSelectionRows()
} else {
multipleSelection.value = val
}
selectObj[active.id] = selectObj[active.id] + 1
} else {
multipleSelection.value = val
}
} else {
multipleSelection.value = val
}
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code appears to be mostly well-written, but there are a few areas that could potentially benefit from improvements:

  1. Select Function Handling: The select function is handling folder permissions differently than non-folder items. It's using TreeToFlatten to flatten nested data structures unnecessarily for folders. This can be optimized by removing this step when it doesn't add value.

  2. Conditional Checks: The conditional checks and logic around the selection of rows feel somewhat repetitive at line 388. Consider simplifying these conditions if necessary.

  3. State Management: Ensure that the state management (multipleSelection, selectObj) is handled correctly within reactive contexts. Using refs might be redundant in some cases where computed properties or store states would suffice.

  4. Error Prone Code: There isn't any obvious error-prone code in this snippet, but ensuring that all variables have been properly initialized before being used can prevent runtime errors.

Here's an updated version with some of these suggestions applied:

@@ -55,7 +55,7 @@
         ref="multipleTableRef"
         class="mt-16"
         :data="filteredData"
-        @selection-change="handleSelectionChange"
+        @select="handleSelect"
         :max-table-height="260"
         :row-key="(row: any) => row.id"
         :expand-row-keys="defaultExpandKeys"
@@ -310,21 +311,35 @@ const filteredData = computed(() => {
 })
 
 const multipleSelection = ref([])
+const selectedFoldersWithAllChildrenSelected = new WeakMap(); // Use WeakMap instead of plain object
+function handleSelect(val: any[], active: any) {
   if (active.resource_type === 'folder' && active.child_count > 0) {
     if (!val.some(item => item.id === active.id)) {
       let childIds = getAllChildIds(active.children);
+      if (!selectedFoldersWithAllChildrenSelected.has(active.id)) {
+        selectedFoldersWithAllChildrenSelected.set(active.id, [...childIds]);
+      }
       console.log(childIds);
-        childIds.forEach(item => {
-          multipleTableRef.value.toggleRowSelection(this.$refs.multipleTableRef.rows.find(row => row.id === item), true);
-        });
+        val.push(...childIds.map(id => multipleTableRef.value.getRowById(id)));
       } else if (selectedFoldersWithAllChildrenSelected.get(active.id).length !== childIds.length) {
         selectedFoldersWithAllChildrenSelected.delete(active.id);
         val.forEach(item => { multipleTableRef.value.toggleRowSelection(item, false); });
+        val.push(...childIds.map(id => multipleTableRef.value.getRowById(id)));
       }
       multipleSelection.value = val;
       return;
     }
   }

+  const expandedFolderIds: string[] = [];
+  Object.entries(selectedFoldersWithAllChildrenSelected).forEach(([key, children]) => {
+    const expandedIndex = multipleTableRef.value.expanded.indexOf(Number(key));
+    if (expandedIndex === -1) {
+      multipleTableRef.value.expandRowByKey(Number(key));
+      expandedFolderIds.push(Number(key));
+    }
+  });

   multipleSelection.value = val.filter((item) => !children.includes(String(item.id)))
}

Explanation Changes:

  • WeakMap Usage: Replaced the simple object with WeakMap to avoid storing references to objects unnecessarily.
  • Expanded Rows Logic: Added logic to expand parent nodes whenever all their child nodes are selected.
  • Simplified Conditional Checks: Eliminated repeated code blocks for checking and toggling selections based on resource type.

}

const dialogVisible = ref(false)
const radioPermission = ref('')
function openMulConfigureDialog() {
Expand Down
Loading