[shaohuzhang1]

fix: home perm

[shaohuzhang1]

Here are the issues and suggestions for the diff:

1. Duplicated permission checks (maintenance risk)
The same permission expressions (perm.application.workspace.create(), etc.) are repeated in both showQuickCreate and each v-if. If you later change a permission, you must update it in two places. Consider deriving a small object or using the computed flags:

const can = computed(() => ({
  application: perm.application.workspace.create(),
  knowledge: perm.knowledge.workspace.create(),
  tool: perm.tool.workspace.create(),
  model: perm.model.workspace.create(),
}))
const showQuickCreate = computed(() => Object.values(can.value).some(Boolean))

Then use v-if="can.application" etc.

2. Potential reactivity issue
perm.application.workspace.create() looks like an imperative call, not necessarily reactive. Calling it inside computed only re-evaluates when its reactive dependencies change. Verify that perm.*.create() reads reactive state internally; otherwise the UI won't update when permissions change (e.g., after login or role switch).

3. Possible early-render flicker
If permissions are loaded asynchronously (e.g., fetched after mount), the section will briefly render empty/hidden before permissions resolve. Ensure permission state is ready before rendering, or add a loading guard.

4. Inconsistent fit-trigger-width casing (pre-existing, but worth noting)
ButtonCreateApplication uses fit-trigger-width (kebab-case attr), while the others use popper-class / popperClass inconsistently across components. Not introduced by this diff, but the reformatting exposes it — verify each component's actual prop names to avoid silently ignored props.

5. Import ordering / lint
import perm from '@/permission' is placed after other imports but grouped loosely. Depending on your ESLint config (import/order), you may want to keep third-party and @/ imports ordered.

6. Heading semantics
<h4 class="mb-4 mt-4">快捷创建</h4> — hard-coded Chinese inline, consistent with existing code but consider i18n if the project supports multiple languages (other strings appear to be through components). Minor.

7. Skipping onMounted fetch when no permissions
onMounted still runs fetching providers/model data regardless of showQuickCreate. If the section is hidden entirely, you could skip those API calls when perm.model.workspace.create() is false, to reduce unnecessary requests.

8. Empty grid layout
When showQuickCreate is true but only some cards show, the xl:grid-cols-4 with 1–2 items is fine, but sm:grid-cols-2 handles it. No action needed, just confirm spacing looks right with fewer items.

9. Prop naming consistency
defaultProvider, providers, ModelApi usage unchanged — fine.

Overall the logic is correct; the main improvements are DRY-ing the permission checks (#1) and verifying reactive permission state (#2).

[shaohuzhang1]

Here's my review:

Potential Issues

1. disabled is both a data property and used in template binding — el-card doesn't have a disabled prop, so that's fine visually, but the disabled state is purely cosmetic. Clicks are guarded in handleNavigate, which is good. However, there's no cursor/opacity feedback distinguishing disabled cards beyond shadow/hover. Consider adding opacity-50 or pointer-events-none (though pointer-events-none would block the click handler entirely, which may be preferable).

2. No visual affordance for disabled state — users may not realize why a card doesn't navigate. Adding a tooltip or dimmed styling would improve UX.

3. Accessibility — @click on el-card isn't keyboard-accessible. Consider wrapping navigation in an <a>/<router-link> for semantics, or add role="button", tabindex="0", and @keydown.enter.

4. Permission checks in a computed — perm.application.workspace.isShare() etc. are called inside a computed. If these functions depend on reactive state, this works; if they read non-reactive globals, the computed won't update when permissions change. Verify reactivity of perm.*.

5. Repeated isSystemScope.value || !perm.X.workspace.isShare() logic — the pattern is repeated 4 times. Consider a small helper like:

   const canShare = (check: () => boolean) => !isSystemScope.value && check()

Minor / Optimization

6. route.meta.scope === 'system' — relies on an untyped meta value. Consider augmenting RouteMeta with a typed scope?: 'system' | ... for safety.

7. handleNavigate(card.route) → handleNavigate(card) — good refactor, since the guard needs the disabled flag.

8. CSS class string — 'cursor-pointer hover:border-current!': !card.disabled uses object syntax correctly. Note the ! suffix is UnoCSS/Tailwind-v4 important syntax; ensure the preprocessor supports it (it was already present before, so fine).

9. disabled optional (?) — left optional means every consumer must handle undefined; since all cards set it, consider making it required to catch omissions at compile time.

10. onMounted(loadResources) — if props.workspaceId changes while mounted (e.g., route param change reusing the component), resources won't reload. Consider watch(() => props.workspaceId, loadResources).

11. Auto-imports — defineProps, ref, computed are auto-imported presumably; perm is a new manual import, consistent with other imports. Fine.

12. Comment placement — <!-- 进入未禁用资源的列表 --> above every card via v-for template is fine, but it's rendered per iteration logically; harmless.

Summary

The diff is functionally sound. Main improvements: add visual/keyboard feedback for disabled cards, centralize the permission predicate, make disabled required, and watch workspaceId for reactivity.

[shaohuzhang1]

Here's my review of the diff:

Summary

The change simply removes a <h4 class="mb-4 mt-4">快捷创建</h4> ("Quick Create") heading from the template. This is a minor, low-risk change, but there are a few things to consider.

Potential Issues

1. Inconsistent section labeling / UX

   • The "资源" (Resources) section still has its heading, but the "快捷创建" (Quick Create) section no longer does. Users may lose context about what the HomeQuickCreate block represents. If the component itself renders its own title, this is fine — otherwise consider keeping a heading or adding one inside the component.

2. Accessibility

   • If the removed <h4> was the only accessible label for the HomeQuickCreate region, screen-reader users lose that landmark context. Verify whether HomeQuickCreate includes an appropriate aria-label, <h2>/<h3>, or role with labeling.

3. Spacing / margin artifacts

   • The removed element had mt-4 mb-4. Check that removing it doesn't break vertical spacing between the page title/MkViewLayout and HomeQuickCreate. You may need to add margin to the component wrapper (or a utility class on HomeQuickCreate) to preserve the intended gap.

4. Empty title="" attribute

   • <MkViewLayout title=""> passes an empty string. If title defaults internally, passing "" explicitly may suppress the default title. Confirm this is intentional; otherwise omit the prop.

5. Comment now slightly misleading

   • The comment <!-- 快捷创建 --> remains but the visible heading is gone. It's harmless, but if the comment was meant to mark the section, ensure it still makes sense.

Optimization Suggestions

• Centralize section headings: Since you have repeated <h4 class="mb-4 mt-4">…</h4> patterns, consider extracting a small <SectionHeading> component or using consistent heading levels (e.g., an <MkSection> wrapper). This avoids drift like the current inconsistency.
• Heading hierarchy: Using <h4> without preceding <h2>/<h3> in the layout can create skipped heading levels. Ensure the document outline is logical for accessibility and SEO.
• Collapse keyword styling: The mb-4 mt-4 utilities could be simplified (e.g., my-4) — minor readability improvement.

Verdict

The diff is safe but should be verified against:

• Whether HomeQuickCreate self-labels (for a11y),
• Spacing consistency after removal,
• The intent behind title="".

No functional bugs are introduced by the change itself.