[shaohuzhang1]

perf: sidebar

[shaohuzhang1]

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.

[shaohuzhang1]

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.

[shaohuzhang1]

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.