Skip to content

perf: sidebar #7922

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
wangdan-fit2cloud merged 1 commit into from
Nov 13, 2026
Merged

perf: sidebar #7922

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
2 changes: 1 addition & 1 deletion ui/src/assets/iconfont.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions ui/src/components/COMPONENT_README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ Esc 或取消清理草稿；`loading` 默认为 false，为 true 时禁止创建

`maxTableHeight` 表示窗口中除表格外需要扣除的高度，默认为 `250`；组件会在窗口尺寸变化时
重新计算 `max-height`。传入 `resizable` 后启用列宽拖拽，并隐藏为拖拽借用的原生边框视觉。
右侧固定列（`fixed="right"`，用于操作栏）统一禁止拖拽调整宽度，其他列保留原有配置。
表格最右侧外边界统一禁止拖拽，即使最后一列不是固定操作列也不开放该边界。
仅开启 `resizable` 时，表头保持单行，拖拽下限按当前标题文字、排序／筛选图标、间距及内边距
计算；拖拽线与最终列宽均不能小于该宽度，原生 `header-dragend` 继续透传。
`resizable` 采用白名单式启用：只有需求明确指定的页面级表格才能开启；未明确指定的表格，以及
Expand Down
8 changes: 6 additions & 2 deletions ui/src/components/global/mk-table/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ const slots = defineSlots<{
'footer-batch-actions'?: () => VNode[]
}>()

/** 表头排序统一交给调用方；在 Element Plus 排序前转换，避免重排当前页。 */
/** 统一列配置：右侧固定操作列禁用调宽，表头排序交给调用方。 */
function normalizeColumn(column: VNode): VNode {
if (column.type === Fragment && Array.isArray(column.children)) {
return h(
Expand All @@ -129,7 +129,10 @@ function normalizeColumn(column: VNode): VNode {
if (column.type !== ElTableColumn) return column

const sortable = column.props?.sortable
const normalizedColumn = cloneVNode(column, sortable === true || sortable === '' ? { sortable: 'custom' } : undefined)
const normalizedColumn = cloneVNode(column, {
...(sortable === true || sortable === '' ? { sortable: 'custom' } : {}),
...(column.props?.fixed === 'right' ? { resizable: false } : {}),
})
const columnSlots = column.children
if (columnSlots && !Array.isArray(columnSlots) && typeof columnSlots === 'object' && typeof columnSlots.default === 'function') {
const defaultSlot = columnSlots.default
Expand Down Expand Up @@ -270,6 +273,7 @@ defineExpose({ clearSelection, tableRef })
:row-key="props.rowKey"
v-bind="$attrs"
:border="props.resizable"
:allow-drag-last-column="false"
@select="handleSelect"
@selection-change="handleSelectionChange"
@pointerdown.capture="handlePointerDown"

@shaohuzhang1 shaohuzhang1 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here are my observations on this diff:

1. Bug: resizable is applied to all right-fixed columns (behavioral change)

...(column.props?.fixed === 'right' ? { resizable: false } : {})

This unconditionally disables resizing for every right-fixed column, not just the operations/action column as the new comment ("右侧固定操作列禁用调宽") implies. If a caller has a legitimate right-fixed data column, it loses resizability without consent.

Suggestions:

  • Gate it on an explicit prop/flag (e.g. a custom disableResize / isActionColumn attribute on the caller's column) rather than inferring it from fixed === 'right'.
  • Or at minimum document this global rule and make it opt-out.
  • Also fixed could be a reactive expression; if column.props.fixed is undefined at normalize time due to binding order, you may miss the case. Consider handling after resolution or via a slot-render check.

2. cloneVNode merge semantics – resizable prop type

  • Element Plus expects resizable as Boolean. { resizable: false } is fine, but note that spreading {} in the non-matching branch means the key is simply absent — good.
  • However, the object spread approach silently overrides any resizable the caller already passed. If a caller sets resizable: true on a right-fixed column, your code wins. Confirm that's intended; otherwise use a fallback (resizable: column.props?.resizable ?? false).

3. allow-drag-last-column="false" — likely wrong value

In Vue templates, a static attribute "false" is passed as the string "false", which is truthy. For a boolean prop this typically coerces to true (or at least does not mean false) depending on casting. You want:

:allow-drag-last-column="false"

otherwise the last (operations) column may still be draggable.

4. Comment vs. code mismatch

The updated doc comment now covers two unrelated concerns: "统一列配置", "右侧固定操作列禁用调宽", and "表头排序交给调用方". The function actually also does slot wrapping for sortable columns. Consider splitting these responsibilities (one helper for sortable normalization, one for fixed/resizable), which improves readability and testability.

5. Potential interaction between sortable and resizable

sortable='custom' combined with resizable:false on the action column is fine, but ensure the header drag-to-sort and column-resize pointer handlers (@pointerdown.capture="handlePointerDown") don't conflict once resize is disabled on a specific column. Worth a manual sanity test.

6. Minor: inline ternary object spread

...(sortable === true || sortable === '' ? { sortable: 'custom' } : {}),

Slightly verbose. Equivalent and clearer:

const clonedProps = {}
if (sortable === true || sortable === '') clonedProps.sortable = 'custom'
if (column.props?.fixed === 'right') clonedProps.resizable = false
const normalizedColumn = cloneVNode(column, clonedProps)

This avoids allocating throwaway {} objects and reads linearly.

7. Missing test coverage implication

The diff adds a silent global behavior (fixed-right → non-resizable). Recommend a unit test asserting:

  • right-fixed action column is non-resizable,
  • other columns retain caller-provided resizable,
  • allow-drag-last-column is actually false at runtime (props unit test).

Summary of actionable fixes

  1. Replace allow-drag-last-column="false" with :allow-drag-last-column="false".
  2. Reconsider hard-coding resizable:false for all right-fixed columns — make it opt-in via a dedicated prop or documented convention.
  3. Align the comment with actual behavior / split the helper.
  4. Add tests for both changes.

Expand Down
8 changes: 6 additions & 2 deletions ui/src/layout/sidebar/SystemSidebar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const getSystemMenuIcon = (systemMenuItem: LayoutMenuItem) =>
:collapse-transition="false"
:default-active="systemActiveMenuPath"
router
:persistent="false"
>
<template v-for="item in systemMenuItems" :key="item.name">
<template v-if="item.permission ? item.permission() : true">
Expand Down Expand Up @@ -129,7 +128,6 @@ const getSystemMenuIcon = (systemMenuItem: LayoutMenuItem) =>

&__title {
border-radius: var(--el-border-radius-base);
color: var(--el-text-color-primary);
height: auto;
line-height: 18px;
padding: 11px calc(var(--spacing) * 2) !important;
Expand All @@ -138,6 +136,12 @@ const getSystemMenuIcon = (systemMenuItem: LayoutMenuItem) =>
background-color: var(--mk-N900-transparent-10);
}
}
&.is-active {
.el-sub-menu__title {
color: var(--el-color-primary);
font-weight: 500;
}
}

.el-icon {
margin-right: calc(var(--spacing) * 2);

@shaohuzhang1 shaohuzhang1 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here are my observations on the diff:

1. Removal of :persistent="false" from <el-menu>

  • Issue: Without persistent, the menu component will keep child sub-menus rendered in memory even when collapsed. This is usually harmless, but if your menus contain dynamic data (e.g., API-fetched counts, permission-dependent items), stale state may persist.
  • Behavior change: With persistent=false, sub-menu contents unmount when closed, so onMounted/onUnmounted hooks and watchers in nested components re-trigger. Verify no child component relies on being kept alive.
  • Optimization: persistent=true improves performance by avoiding re-renders; ensure this removal was intentional (likely to avoid memory leaks or refresh issues).

2. Color variable removal from &__title

  • Issue: Removing color: var(--el-text-color-primary) means non-active sub-menu titles now rely on inherited color from el-menu. If el-sub-menu__title has its own default in Element Plus (it does — usually --el-menu-text-color), that's fine, but if you were overriding a custom theme, the inheritance chain may now resolve differently.
  • Suggestion: Confirm the base text color still resolves correctly (especially against dark modes / custom themes). Otherwise add an explicit fallback.

3. New &.is-active rule

  • Potential issue — selector specificity/scoping: The new rule targets .is-active > .el-sub-menu__title. In Vue scoped styles, deep child selectors need :deep() (or ::v-deep) to pierce into Element Plus internals. If this block lives inside a <style scoped>, the .el-sub-menu__title selector will not match unless wrapped with :deep(...). Same applies to the existing &:hover .el-sub-menu__title rule above it — worth double-checking both.
  • Correctness concern: Is is-active applied to the parent <el-sub-menu>? Element Plus adds is-active to sub-menu wrappers when a descendant item is active, so this should work — but verify the class lands where expected in your DOM (some versions apply is-active only on li.el-menu-item, not on sub-menu containers).
  • Missing hover override: When active, hovering a sub-menu title will still trigger the &:hover background var(--mk-N900-transparent-10). Consider whether active + hover should have distinct styling.

Minor suggestions

  • The 300 → 500 font-weight change could cause layout shift if fonts aren't loaded at weight 500; ensure the weight is available in your font stack.
  • Consider extracting these magic spacing values (calc(var(--spacing) * 2)) into local tokens for maintainability.

Summary: The most likely real bug is selector scoping (:deep()) for the new .is-active .el-sub-menu__title rule; the persistent removal is a behavior change worth confirming is intentional.

Expand Down
6 changes: 6 additions & 0 deletions ui/src/router/admin/system/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const systemRoutes: RouteRecordRaw = {
meta: {
title: '身份与权限',
icon: 'icon_account_outlined',
activeIcon: 'icon_account_filled',
order: 10,
permission: () => {
return (
Expand Down Expand Up @@ -148,6 +149,8 @@ export const systemRoutes: RouteRecordRaw = {
meta: {
title: '资源管理',
icon: 'icon_project-settings-outlined',
activeIcon: 'icon_project-settings-filled',

order: 20,
resourceScope: 'system-resource',
permission: () => {
Expand Down Expand Up @@ -190,6 +193,7 @@ export const systemRoutes: RouteRecordRaw = {
meta: {
title: '共享资源',
icon: 'icon_folder-share_outlined',
activeIcon: 'icon_folder-share_filled',
order: 30,
resourceScope: 'system-shared',
permission: () => {
Expand Down Expand Up @@ -228,6 +232,7 @@ export const systemRoutes: RouteRecordRaw = {
meta: {
title: '对话端管理',
icon: 'icon_contacts_outlined',
activeIcon: 'icon_contacts_filled',
order: 40,
permission: () => {
return perm.system.chatUser.read() || perm.system.chatUserGroup.read() || perm.system.chatAuth.read() || perm.system.portal.read()
Expand Down Expand Up @@ -268,6 +273,7 @@ export const systemRoutes: RouteRecordRaw = {
meta: {
title: '系统设置',
icon: 'icon_setting',
activeIcon: 'icon_setting_filled',
order: 50,
permission: () => {
return perm.system.loginAuth.read() || perm.system.appearance.read() || perm.system.email.read()

@shaohuzhang1 shaohuzhang1 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Irregularities / Potential issues

  1. Stray blank line (lines around the "资源管理" meta):

    activeIcon: 'icon_project-settings-filled',
+
order: 20,

    Remove the extra blank line to keep formatting consistent with other blocks.

  2. Inconsistent icon naming conventions:

    • icon_project-settings-outlined / icon_project-settings-filled uses a hyphen.
    • Others use underscores (icon_account_outlined, icon_contacts_filled).
      Verify these names actually exist in your icon set — a typo here silently fails to render the active state.

  3. 系统设置 uses base icons icon_setting / icon_setting_filled. Ensure icon_setting_filled exists; if the pattern is icon_<name>_outlined, this entry breaks the convention and may need to be icon_setting_outlined.

  4. No type safety on activeIcon: since this is added to meta for every route, consider extending the route meta type (e.g., declare module augmentation for RouteMeta) so missing/incorrect activeIcon values are caught at compile time rather than silently ignored.

  5. Repetition across all route metas suggests room for optimization: consider deriving activeIcon from icon (e.g., replace -outlined/_outlined → _filled) via a helper, or centralizing icon mapping, to avoid future mismatches.

  6. Verify the active-state logic somewhere consumes activeIcon (menu renderer). If not implemented yet, these additions are dead metadata.

Summary: main concrete issue is the stray blank line; everything else is consistency/typing/maintainability advice.

Expand Down
Loading