Skip to content

fix: support click model card to edit #7870

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
fit2cloudwxx merged 1 commit into from
Nov 12, 2026
Merged

fix: support click model card to edit #7870

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
48 changes: 36 additions & 12 deletions ui/src/views/model/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script setup lang="ts">
import { computed, onMounted, ref } from 'vue'
import { computed, nextTick, onMounted, ref, useTemplateRef } from 'vue'
import type { Dict, ModelItem, ModelProviderItem, OptionItem } from '@/api/types'
import { MODEL_TYPE_LABELS } from '@/constants'
import CommonApi from '@/api/admin/workspace/common'
Expand All @@ -11,13 +11,8 @@ import RelatedResourcesApi from '@/api/admin/workspace/related-resources'
import ModelCard from './model-card/ModelCard.vue'
import ButtonAddModel from './create-model/ButtonAddModel.vue'
import ModelProvider from './components/ModelProvider.vue'
import {
AuthorizeModelAction,
DeleteModelAction,
EditModelAction,
ParamSettingAction,
RelatedResourcesModelAction,
} from './model-card/action-dropdown'
import EditModelDrawer from './model-card/action-dropdown/edit-model-action/EditModelDrawer.vue'
import { AuthorizeModelAction, DeleteModelAction, ParamSettingAction, RelatedResourcesModelAction } from './model-card/action-dropdown'
import perm from '@/permission/index.ts'

const DEFAULT_MODEL_PROVIDER: ModelProviderItem = { icon: '', name: '全部模型', provider: 'all' }
Expand Down Expand Up @@ -84,6 +79,17 @@ function handleProviderSelect(provider: ModelProviderItem) {
loadModels()
}

/* 卡片与菜单共用编辑抽屉，沿用模型编辑权限。 */
const editDrawerMounted = ref(false)
const editModelDrawerRef = useTemplateRef<InstanceType<typeof EditModelDrawer>>('editModelDrawerRef')

async function handleOpenEditModel(model: ModelItem) {
if (isShared.value || !perm.model.workspace.modify(model.id) || editDrawerMounted.value) return
editDrawerMounted.value = true
await nextTick()
editModelDrawerRef.value?.open(getModelProvider(model), model)
}

onMounted(() => {
Promise.all([loadModelProviders(), loadModels()])
})
Expand All @@ -100,7 +106,13 @@ onMounted(() => {
<h4>{{ currentProvider.name }}</h4>
<div class="flex-align-center">
<MkComplexSearch :fields="searchFields" @change="handleSearchChange" />
<ButtonAddModel v-if="!isShared && perm.model.workspace.create()" :current-provider="currentProvider" :providers="modelProviders" @refresh="loadModels" :api="ModelApi" />
<ButtonAddModel
v-if="!isShared && perm.model.workspace.create()"
:current-provider="currentProvider"
:providers="modelProviders"
@refresh="loadModels"
:api="ModelApi"
/>
</div>
</component>
<div v-loading="loading">
Expand All @@ -113,16 +125,27 @@ onMounted(() => {
:refresh="loadModels"
:shared="isShared"
:disabled="isShared"
@click="handleOpenEditModel(model)"
>
<template #action-dropdown>
<!-- 编辑 -->
<EditModelAction v-if="perm.model.workspace.modify(model.id)" label="编辑" :api="ModelApi" :model="model" :provider="getModelProvider(model)" @refresh="loadModels" />
<MkAction v-if="perm.model.workspace.modify(model.id)" label="编辑" icon="icon_edit_outlined" @click="handleOpenEditModel(model)" />
<!-- 模型参数设置 -->
<ParamSettingAction v-if="model.model_type !== 'RERANKER' && perm.model.workspace.paramSetting(model.id)" label="模型参数设置" :api="ModelApi" :model="model" />
<ParamSettingAction
v-if="model.model_type !== 'RERANKER' && perm.model.workspace.paramSetting(model.id)"
label="模型参数设置"
:api="ModelApi"
:model="model"
/>
<!-- 资源授权 -->
<AuthorizeModelAction v-if="perm.model.workspace.auth(model.id)" label="资源授权" :model="model" />
<!-- 查看关联资源 -->
<RelatedResourcesModelAction v-if="perm.model.workspace.relateMap(model.id)" label="查看关联资源" :api="RelatedResourcesApi" :model="model" />
<RelatedResourcesModelAction
v-if="perm.model.workspace.relateMap(model.id)"
label="查看关联资源"
:api="RelatedResourcesApi"
:model="model"
/>
<!-- 删除 -->
<DeleteModelAction v-if="perm.model.workspace.delete(model.id)" label="删除" :api="ModelApi" :model="model" @refresh="loadModels" />
</template>
Expand All @@ -133,6 +156,7 @@ onMounted(() => {
</div>
</template>
</MkViewLayout>
<EditModelDrawer v-if="editDrawerMounted" ref="editModelDrawerRef" :api="ModelApi" @closed="editDrawerMounted = false" @refresh="loadModels" />
</template>

<style scoped lang="scss"></style>

@shaohuzhang1 shaohuzhang1 Nov 12, 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.

The diff is structurally reasonable: it replaces a per-card edit action with one shared EditModelDrawer, and the editDrawerMounted guard prevents duplicate drawer mounts. However, there are several issues to verify/fix.

1. Event bubbling is the main risk

<ModelCard
  ...
  @click="handleOpenEditModel(model)"
>

Adding @click to the whole card will also receive clicks from interactive children inside the card, especially the action-dropdown trigger and any buttons/icons. That means:

  • clicking the ... / action menu trigger may also open the edit drawer;
  • clicking other dropdown actions may trigger the card edit handler if the dropdown is not teleported or does not stop propagation;
  • a single click may cause two actions.

Fix by either:

  • removing the whole-card click and using a dedicated edit button/area, or
  • adding @click.stop to the action-dropdown trigger and all interactive controls inside ModelCard.

Also verify that ModelCard has a single root element. If it is a fragment/multi-root component, the native click listener may not fall through as expected.

2. Verify the EditModelDrawer contract

The new code assumes that EditModelDrawer:

  • exposes an open(provider, model) method via defineExpose;
  • emits closed;
  • emits refresh;
  • accepts the api prop.

If any of these is missing or named differently, the drawer may never open, never unmount, or never refresh the list.

If the drawer uses v-model / update:modelValue instead of closed, then editDrawerMounted will never be reset. That would permanently block future edit attempts.

3. editDrawerMounted can get stuck

editDrawerMounted.value = true
await nextTick()
editModelDrawerRef.value?.open(getModelProvider(model), model)

If the ref is null, or if open() throws, editDrawerMounted remains true, and all future edit clicks are ignored. Use a safer version:

async function handleOpenEditModel(model: ModelItem) {
  if (
    isShared.value ||
    !perm.model.workspace.modify(model.id) ||
    editDrawerMounted.value
  ) {
    return
  }

  editDrawerMounted.value = true
  await nextTick()

  const drawer = editModelDrawerRef.value
  if (!drawer) {
    editDrawerMounted.value = false
    return
  }

  try {
    await drawer.open(getModelProvider(model), model)
  } catch (error) {
    editDrawerMounted.value = false
    throw error
  }
}

If open() is synchronous, the try/catch is still useful; await can be removed if it is not async.

4. useTemplateRef requires Vue 3.5+

const editModelDrawerRef = useTemplateRef<
  InstanceType<typeof EditModelDrawer>
>('editModelDrawerRef')

This is fine on Vue 3.5+, but will not work on older Vue 3 versions. Confirm the project version. If the project is on Vue < 3.5, use:

const editModelDrawerRef = ref<InstanceType<typeof EditModelDrawer> | null>(null)

5. Verify MkAction is available

<MkAction
  v-if="perm.model.workspace.modify(model.id)"
  label="编辑"
  icon="icon_edit_outlined"
  @click="handleOpenEditModel(model)"
/>

Since EditModelAction was removed, verify that MkAction is globally registered or auto-imported. Also verify that it accepts the icon prop and that clicking it closes the dropdown as expected.

6. Accessibility / UX concern

A whole card with @click is not keyboard-accessible unless it has proper role, tabindex, and keydown.enter / keydown.space handling. A dedicated edit button inside the dropdown is safer and clearer. At minimum, avoid making the card look clickable when the user lacks modify permission or when isShared is true.

7. Minor optimization

The formatting-only changes to ButtonAddModel, ParamSettingAction, and RelatedResourcesModelAction increase diff noise. They are harmless, but can be omitted to keep the review focused.

Summary

The biggest issue is the whole-card @click combined with an action dropdown inside the card. Add @click.stop on interactive card children or use a dedicated edit trigger. Then verify the drawer exposes open, emits closed/refresh, and that editDrawerMounted is reset if opening fails.

16 changes: 15 additions & 1 deletion ui/src/views/model/model-card/ModelCard.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const props = defineProps<{
shared: boolean
}>()

const emit = defineEmits<{ click: [event: MouseEvent] }>()

defineSlots<{ 'action-dropdown'?: () => unknown }>()

const errMessage = computed(() => {
Expand All @@ -30,10 +32,22 @@ const errMessage = computed(() => {
}
return ''
})

function handleClick(event: MouseEvent) {
if (props.disabled) return
emit('click', event)
}
</script>

<template>
<MkResourceCard class="relative overflow-hidden" :title="model.name" :nick_name="model.nick_name || '-'" :create_time="model.create_time">
<MkResourceCard
class="relative overflow-hidden"
:disabled="disabled"
:title="model.name"
:nick_name="model.nick_name || '-'"
:create_time="model.create_time"
@click="handleClick"
>
<template #icon>
<span class="block h-6 w-6" :innerHTML="provider.icon" />
</template>

@shaohuzhang1 shaohuzhang1 Nov 12, 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 is an analysis of the code diff provided:

1. Potential Issue: HTML Injection Risk

Location: <template> section

<span class="block h-6 w-6" :innerHTML="provider.icon" />

Issue: Using v-html (or its shorthand :innerHTML) renders raw HTML. If provider.icon contains untrusted user input or comes from a database without strict sanitization, this creates a Cross-Site Scripting (XSS) vulnerability.
Suggestion: Ensure provider.icon is strictly sanitized or use an SVG component/icon library instead of injecting raw HTML strings.

2. Typo/Vue 3 API Consistency

Location: <script setup> and <template>
Issue: The code uses innerHTML. In Vue templates with v-bind, you should use innerHTML (DOM property) which works, but it's less common than using v-html. However, the bigger "irregularity" is stylistic.
Note: While :innerHTML works, standard Vue practice for dynamic HTML is v-html="...". If you stick with binding, ensure your linter allows it. More importantly, checking if MkResourceCard actually handles the @click event properly is crucial.

3. Missing Prop Definition for disabled

Location: Template binding :disabled="disabled"
Issue: You are passing :disabled="disabled" to MkResourceCard, but looking at defineProps in the snippet, disabled is not shown as being defined in props.
Check: Ensure disabled is explicitly declared in defineProps<{ ... }>(). If it’s not defined, Vue will treat it as an attr (fallthrough) instead of a prop, and TypeScript may complain if strict checking is enabled.
Current snippet shows:

const props = defineProps<{
   shared: boolean
}>()

Fix: Add disabled: boolean to the props interface if used.

4. Event Type Optimization

Location: defineEmits

const emit = defineEmits<{ click: [event: MouseEvent] }>()

Observation: This is a good, modern Vue 3 pattern for type-safe emits. No issues here.

5. Logic Check: handleClick Guard

Location: Script logic

function handleClick(event: MouseEvent) {
  if (props.disabled) return
  emit('click', event)
}

Optimization/Suggestion:
While guarding inside the handler prevents the event from propagating in logic, many UI component libraries (like Element Plus, Ant Design Vue, etc.) automatically ignore clicks when disabled is true on root elements or apply pointer-events: none via CSS.

  • If MkResourceCard handles disabled state internally: Your JS guard might be redundant but safe.
  • If UI consistency is priority: Ensure that visual styling (cursor changes, opacity) also reflects the disabled state. Relying solely on JS prevention while the element looks clickable can lead to poor UX.

Summary of Recommendations

  1. Security: Verify provider.icon source. If external/untrusted, sanitize it or avoid innerHTML.
  2. Completeness: Confirm disabled is included in defineProps.
  3. UI/UX: Ensure MkResourceCard has appropriate visual feedback for the disabled state (e.g., opacity, cursor-not-allowed).
  4. Vue Best Practice: Consider using v-html="provider.icon" instead of :innerHTML for better readability and standard template syntax recognition by tools/linters.

Revised Template Snippet (for style):

<span class="block h-6 w-6" v-html="provider.icon" />

Expand Down
48 changes: 36 additions & 12 deletions ui/src/views/system/shared-resources/model/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script lang="ts" setup>
import AuthorizedWorkspaceAction from '../authorized-workspace-action/index.vue'
import { computed, onMounted, ref } from 'vue'
import { computed, nextTick, onMounted, ref, useTemplateRef } from 'vue'
import type { Dict, ModelItem, ModelProviderItem, OptionItem } from '@/api/types'
import { MODEL_TYPE_LABELS } from '@/constants'
import SystemSharedModelApi from '@/api/admin/system/shared-resources/model'
Expand All @@ -10,7 +10,8 @@ import ProviderApi from '@/api/admin/model-provider'
import ModelCard from '@/views/model/model-card/ModelCard.vue'
import ButtonAddModel from '@/views/model/create-model/ButtonAddModel.vue'
import ModelProvider from '@/views/model/components/ModelProvider.vue'
import { DeleteModelAction, EditModelAction, ParamSettingAction, RelatedResourcesModelAction } from '@/views/model/model-card/action-dropdown'
import { DeleteModelAction, ParamSettingAction, RelatedResourcesModelAction } from '@/views/model/model-card/action-dropdown'
import EditModelDrawer from '@/views/model/model-card/action-dropdown/edit-model-action/EditModelDrawer.vue'
import perm from '@/permission'
import { RESOURCE_TYPE } from '@/api/enums'

Expand Down Expand Up @@ -73,6 +74,17 @@ function handleProviderSelect(provider: ModelProviderItem) {
loadModels()
}

/* 卡片与菜单共用编辑抽屉，沿用共享模型编辑权限。 */
const editDrawerMounted = ref(false)
const editModelDrawerRef = useTemplateRef<InstanceType<typeof EditModelDrawer>>('editModelDrawerRef')

async function handleOpenEditModel(model: ModelItem) {
if (!perm.model.share.modify() || editDrawerMounted.value) return
editDrawerMounted.value = true
await nextTick()
editModelDrawerRef.value?.open(getModelProvider(model), model)
}

onMounted(() => {
Promise.all([loadModelProviders(), loadModels()])
})
Expand Down Expand Up @@ -102,17 +114,17 @@ onMounted(() => {
<div v-loading="loading">
<div v-if="ModelItems.length" class="mk-resource-card-grid">
<template v-for="model in ModelItems" :key="model.id">
<ModelCard :api="SystemSharedModelApi" :model="model" :provider="getModelProvider(model)" :refresh="loadModels" :shared="true">
<ModelCard
:api="SystemSharedModelApi"
:model="model"
:provider="getModelProvider(model)"
:refresh="loadModels"
:shared="true"
@click="handleOpenEditModel(model)"
>
<template #action-dropdown>
<!-- 编辑 -->
<EditModelAction
v-if="perm.model.share.modify()"
label="编辑"
:api="SystemSharedModelApi"
:model="model"
:provider="getModelProvider(model)"
@refresh="loadModels"
/>
<MkAction v-if="perm.model.share.modify()" label="编辑" icon="icon_edit_outlined" @click="handleOpenEditModel(model)" />
<!-- 模型参数设置 -->
<ParamSettingAction
v-if="model.model_type !== 'RERANKER' && perm.model.share.paramSetting()"
Expand All @@ -121,7 +133,12 @@ onMounted(() => {
:model="model"
/>
<!-- 授权工作空间 -->
<AuthorizedWorkspaceAction v-if="perm.model.share.authToWorkspace()" label="授权工作空间" :resource="model" :resource-type="RESOURCE_TYPE.MODEL" />
<AuthorizedWorkspaceAction
v-if="perm.model.share.authToWorkspace()"
label="授权工作空间"
:resource="model"
:resource-type="RESOURCE_TYPE.MODEL"
/>

<!-- 查看关联资源 -->
<RelatedResourcesModelAction v-if="perm.model.share.relateMap()" label="查看关联资源" :api="RelatedResourcesApi" :model="model" />
Expand All @@ -136,4 +153,11 @@ onMounted(() => {
</div>
</template>
</MkViewLayout>
<EditModelDrawer
v-if="editDrawerMounted"
ref="editModelDrawerRef"
:api="SystemSharedModelApi"
@closed="editDrawerMounted = false"
@refresh="loadModels"
/>
</template>
2 changes: 1 addition & 1 deletion ui/src/views/tool/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ const router = useRouter()
const editToolFormRef = useTemplateRef<InstanceType<typeof EditToolForm>>('editToolFormRef')

function handleOpenEditTool(tool: ToolItem) {
if (!perm.tool.workspace.read()) return
if (!perm.tool.workspace.edit(tool.id)) return
return editToolFormRef.value?.open(tool)
}

Expand Down
Loading