feat: Optimize the menu Role Permission page#43
Conversation
WalkthroughThis update refactors several Vue components related to menu, permission, and role management. It introduces new localization keys, restructures menu and permission editing to use grids with inline editing, removes modal-based update flows for roles and permissions, and updates UI elements for consistency. The update also removes the dedicated role update component, consolidating editing logic into grid-based interfaces. Additionally, menu tree handling changes from a tree component to a grid with tree configuration, and role permission selection UI is enhanced with filtering and icon support. Changes
Sequence Diagram(s)Inline Role Update Flow (New)sequenceDiagram
participant User
participant RoleTable (Grid)
participant API
participant Modal
User->>RoleTable: Edit role cell (name or permissions)
RoleTable->>API: updateRole(editedRow)
API-->>RoleTable: Success/Error response
RoleTable->>Modal: Show success/error message
RoleTable->>Parent: emit updateRoleClose
Inline Permission Update Flow (New)sequenceDiagram
participant User
participant PermissionGrid
participant API
participant Modal
User->>PermissionGrid: Edit permission cell (name or desc)
PermissionGrid->>API: updatePermission(editedRow)
API-->>PermissionGrid: Success/Error response
PermissionGrid->>Modal: Show success/error message
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
template/tinyvue/src/views/menu/info/components/menu-tree.vue (1)
66-72: Remove dead code (localeDataprop &treeOpcomputed)
localeDataandtreeOpare never referenced in the template or script. Keeping unused reactives increases bundle size and cognitive load.- const props = defineProps<{ - data: ITreeNodeData[]; - localeData: { value: string; label: string }[]; - }>(); + const props = defineProps<{ + data: ITreeNodeData[]; + }>(); - const treeOp = computed(() => ({ data: props.data }));template/tinyvue/src/views/role/info/components/role-table.vue (6)
2-2: Remove unused importThe
nextTickis imported but never used in this component. Remove it to keep imports clean.-import { nextTick, reactive, ref, watch } from 'vue'; +import { reactive, ref, watch } from 'vue';
37-37: Remove unused reactive variableThe
roleTablereactive variable is initialized but never used throughout the component.-const roleTable = reactive([]);
74-82: Optimize permission name concatenationThe current string concatenation approach can be simplified using array methods for better readability and performance.
const getPermission = (row:any) => { - let permissionDate = '' - if (row?.permission.length) { - row?.permission.forEach((item, index) => { - permissionDate = `${permissionDate} ${row?.permission[index].name}` - }); - } - return permissionDate + if (!row?.permission?.length) return ''; + return row.permission.map(item => item.name).join(' '); };
160-168: Add accessibility improvements for icon usageThe icon components are used without any accessibility attributes. Consider adding aria labels or tooltips.
-<iconCueL class="del-icon"></iconCueL> +<iconCueL class="del-icon" aria-label="Bind role" title="Bind role"></iconCueL> ... -<iconDel class="del-icon"></iconDel> +<iconDel class="del-icon" aria-label="Delete role" title="Delete role"></iconDel>
171-171: Fix inconsistent CSS class namingThe delete operation uses the class
operation-updatewhich is misleading. It should useoperation-deleteto maintain clarity and consistency with its function.-class="operation-update" +class="operation-delete"
197-203: Fix CSS formatting issuesThere are formatting issues in the CSS including a double semicolon and inconsistent spacing.
.del-icon{ fill: #1890ff; margin-right: 8px; - margin-left: 16px;; + margin-left: 16px; font-size: 16px; margin-top: -3px; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
template/nestJs/locales.json(2 hunks)template/tinyvue/src/views/menu/info/components/info-tab.vue(2 hunks)template/tinyvue/src/views/menu/info/components/menu-tree.vue(1 hunks)template/tinyvue/src/views/permission/info/components/info-tab.vue(1 hunks)template/tinyvue/src/views/role/info/components/add-role.vue(1 hunks)template/tinyvue/src/views/role/info/components/info-tab.vue(5 hunks)template/tinyvue/src/views/role/info/components/menu-drawer.vue(1 hunks)template/tinyvue/src/views/role/info/components/permission-table.vue(1 hunks)template/tinyvue/src/views/role/info/components/role-table.vue(5 hunks)template/tinyvue/src/views/role/info/components/update-role.vue(0 hunks)
💤 Files with no reviewable changes (1)
- template/tinyvue/src/views/role/info/components/update-role.vue
🔇 Additional comments (11)
template/tinyvue/src/views/role/info/components/permission-table.vue (1)
8-11: Column order changed to improve UI layout.The name column is now positioned before the description column, which provides a more logical viewing order when users scan the permission table.
template/tinyvue/src/views/role/info/components/add-role.vue (2)
100-100: UI enhancement: Added rounded styling to primary button.The rounded styling improves visual consistency with other buttons in the role and permission management interfaces.
103-103: UI enhancement: Added rounded styling to cancel button.The rounded styling on the cancel button matches the confirm button and maintains consistent UI design throughout the application.
template/nestJs/locales.json (2)
593-593: Added English localization key for search placeholder.New localization key
menu.add.placeholderwith English translation supports search input functionality in the menu management UI.
1201-1201: Added Chinese localization key for search placeholder.Corresponding Chinese translation for the
menu.add.placeholderkey ensures consistent internationalization support.template/tinyvue/src/views/menu/info/components/info-tab.vue (2)
98-98: Updated parameter handling in onUpdate function.Changed from destructured
{ data }to directdata: Nodeparameter to align with updated event payload structure from the child component.
260-260: Added localeData prop binding to menu-tree component.This provides locale information to the menu-tree component, enabling proper localization support for menu items in the tree display.
template/tinyvue/src/views/menu/info/components/menu-tree.vue (1)
25-30: Possible duplicate icon renderingThe
customIconcolumn already displays the icon (<component :is="row.customIcon">).
Rendering the same icon again in the Operations column (line 35) may be unintentional noise.Confirm if the second icon is required; otherwise remove it to declutter the grid.
template/tinyvue/src/views/permission/info/components/info-tab.vue (1)
130-139: Form validation rules reference non-existent propsRules reference
nameanddesc, but the form items useprop="desc"andprop="name"in reverse order (desc first).
Although both fields are required, clarify to avoid accidental mismatches when extending rules.No code change needed; just verify.
template/tinyvue/src/views/role/info/components/role-table.vue (2)
119-122: Good implementation of inline editingThe use of cell-based editing with trigger on click is a good approach for inline editing. This aligns with the refactoring goal of removing the modal workflow.
139-157: Well-structured permission selection fieldThe implementation of the permission selection using TinySelect with multiple selection support is a good approach. The configuration is comprehensive with proper value and text field mappings.
|
@wuyiping0628 主要有以下问题：
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
template/tinyvue/src/views/menu/info/components/update-form.vue (2)
70-70: Consider adding error handling for missing icons data.The icons import assumes the JSON file exists and has the expected structure. Consider adding error handling or type checking to prevent runtime errors.
-import { icons } from '@opentiny/icons/json/icons.json'; +import { icons } from '@opentiny/icons/json/icons.json'; + +// Add type safety and error handling +const safeIcons = icons || {};
130-136: Optimize icon data generation for better performance.The current implementation creates a new
iconDatasarray with rendered elements on every component instantiation, which could impact performance with a large number of icons.Consider memoizing the icon data or moving it outside the component:
-const iconDatas = Object.keys(icons).map((key) => { - return { - label: key, - value: key, - icon: h('i', { class: `ci-${key}`, style: { fontSize: '18px', marginRight: '6px' } }), - }; -}); +const iconDatas = computed(() => + Object.keys(icons).map((key) => ({ + label: key, + value: key, + icon: h('i', { + class: `ci-${key}`, + style: { fontSize: '18px', marginRight: '6px' }, + 'aria-label': key // Add accessibility + }), + })) +);This change:
- Uses
computed()for better reactivity management- Adds
aria-labelfor accessibility- Fixes spacing in the style object
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
template/nestJs/locales.json(6 hunks)template/tinyvue/src/views/menu/info/components/add-menu.vue(0 hunks)template/tinyvue/src/views/menu/info/components/info-tab.vue(6 hunks)template/tinyvue/src/views/menu/info/components/menu-tree.vue(2 hunks)template/tinyvue/src/views/menu/info/components/update-form.vue(3 hunks)template/tinyvue/src/views/permission/info/components/info-tab.vue(1 hunks)template/tinyvue/src/views/role/info/components/add-role.vue(2 hunks)template/tinyvue/src/views/role/info/components/info-tab.vue(6 hunks)template/tinyvue/src/views/role/info/components/role-table.vue(5 hunks)
💤 Files with no reviewable changes (1)
- template/tinyvue/src/views/menu/info/components/add-menu.vue
🚧 Files skipped from review as they are similar to previous changes (7)
- template/tinyvue/src/views/role/info/components/add-role.vue
- template/tinyvue/src/views/permission/info/components/info-tab.vue
- template/tinyvue/src/views/menu/info/components/menu-tree.vue
- template/nestJs/locales.json
- template/tinyvue/src/views/role/info/components/info-tab.vue
- template/tinyvue/src/views/menu/info/components/info-tab.vue
- template/tinyvue/src/views/role/info/components/role-table.vue
🔇 Additional comments (2)
template/tinyvue/src/views/menu/info/components/update-form.vue (2)
41-50: LGTM! Icon selection form item is well implemented.The new icon selection form item correctly uses a filterable select with proper placeholder and validation setup. The implementation follows the established pattern used by other form items in this component.
68-68:Details
✅ Verification successful
Verify the
hfunction import is necessary.The
hfunction is imported but should be confirmed as necessary for the icon rendering in theiconDatasarray.
🏁 Script executed:
#!/bin/bash # Description: Verify if the h function is actually used in the component # Expected: Should find usage of h() function in the iconDatas array rg -A 5 -B 5 "h\(" template/tinyvue/src/views/menu/info/components/update-form.vueLength of output: 618
No changes needed for
himportThe
hfunction is used in theiconDatasarray to render the<i>elements for icons, so the import should be kept.
2 participants
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor
Style
Chores