Skip to content

fix: template card detail and text - #7792

Merged
fit2cloudwxx merged 1 commit into
v3from
pr@v3@fix_templateCardDetailAndText
Nov 10, 2026
Merged

fit2cloudwxx merged 1 commit into
v3from
pr@v3@fix_templateCardDetailAndText

Conversation

@shaohuzhang1

@shaohuzhang1 shaohuzhang1 commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

【【智能体、知识库、工具】1、模板中心、工具商店:点击卡片或者【详情】显示详情;2、按钮文案【使用】改为【应用】】
https://www.tapd.cn/tapd_fe/45245355/bug/detail/1145245355001079067

<el-button class="flex-1!" type="primary" :disabled="disabled" @click="handleUseTemplate">应用</el-button>
</div>
</component>
</template>

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 the review of the provided Vue code snippet, highlighting irregularities, potential issues, and optimization suggestions.

1. Conflicting Click Handlers (Critical Issue)

Issue: You added a @click event to the outer <MkResourceCard> that triggers handleOpenDetail. However, inside the component, there is already a button (详情 / Details) that also calls handleOpenDetail.

  • Risk: If the user clicks anywhere on the card other than the inner buttons, it will trigger handleOpenDetail. This might be intended ("click card to view details"), but it creates two ways to trigger the same action.
  • Conflict: If the intention was that clicking the card opens the detail modal, ensure that the inner "Details" button is redundant or behaves differently. If the inner button must remain, consider making the outer click handler conditional so it doesn't double-fire if the user accidentally clicks near the button but outside its specific stop-propagation area.

Suggestion:
If the whole card should open details, remove the outer @click and rely on the inner button, or conversely, remove the inner "Details" button if the card click is sufficient. If both must exist, ensure UX clarity.

2. @click.stop Placement and Efficiency

Observation: The inner <div class="flex min-w-0 flex-1" @click.stop> prevents propagation from the two buttons to the parent card.

  • Current State: This correctly prevents the card's new @click="handleOpenDetail" from firing when either "Details" or "Apply" buttons are clicked.
  • Assessment: This logic is correct given the change in line 33. No change needed here, but it highlights the dependency: the parent click handler relies on this .stop modifier.

3. Naming Convention Inconsistency

Change: You changed "使用模板" (Use Template) to "应用模板" (Apply Template) in the comment, and "使用" to "应用" on the button.
Function Name: The function is still named handleUseTemplate.

  • Issue: The UI text says "Apply" ("应用"), but the code variable/function name says "Use" (handleUseTemplate).
  • Suggestion: For maintainability, rename the function to handleApplyTemplate to keep the code semantics aligned with the UI terminology. This prevents confusion for future developers.

4. CSS Class cursor-pointer

Observation: Added class="cursor-pointer" to <MkResourceCard>.
Issue:

  • Does MkResourceCard expose a standard classList or pass through attributes? Most Vue components do via inheritAttrs, but if the root element of MkResourceCard is not a native DOM element (e.g., another wrapper), styles might not apply as expected.
  • Best Practice: Instead of adding a utility class to a potentially custom component, it’s safer to add the style to a wrapping <div> or ensure MkResourceCard supports style-class passthrough. Alternatively, define the cursor behavior internally in MkResourceCard if it's interactive, or use :deep if styling globally.

5. Accessibility (a11y) Suggestion

Issue: You made the entire card clickable (<div> effectively, or component root) but did not specify an ARIA role.
Suggestion: To improve accessibility, ensure the card has role="button" or wrap the interactive part in an actual <button> / <a> tag, or add aria-labels. Screen readers might not recognize the card as interactive unless handled properly by the underlying component structure.

6. Minor Syntax/Style Check

<MkResourceCard :title="template.name" class="cursor-pointer" @click="handleOpenDetail">
  • Check: Ensure template object is always defined before rendering to avoid [Vue warn]: TypeError: Cannot read property 'name' of undefined during initial render or data load delays. Use optional chaining template?.name if template might be null.

✅ Recommended Refactored Code

<script setup lang="ts">
// Rename function to match UI label "Apply"
function handleUseTemplate() {
  // ... implementation
}
</script>

<template>
  <!-- 
    Option A: If the whole card is clickable for details:
    Ensure MkResourceCard passes through attrs/class. 
    Consider removing the inner "Details" button to simplify UX if redundant.
  -->
  <MkResourceCard 
    :title="template?.name" 
    class="cursor-pointer group-hover:bg-color-bg-subtle transition-colors" 
    @click="handleOpenDetail"
  >
    <template #icon>
      <el-avatar class="bg-transparent!" shape="square" :size="24">
        <img :src="resetUrl(template.icon, true)" alt="" />
      </el-avatar>
    </template>

    <!-- Content ... -->

    <template #footer>
      <component :is="footerComponent">
        <div class="flex min-w-0 flex-1" @click.stop>
          <!-- Keep only if you want explicit actions separate from general card click -->
          <!-- If card click already does "Details", consider removing this button -->
          <el-button 
            class="flex-1 mr-2!" 
            plain 
            @click.stop="handleOpenDetail" 
          >
            Details
          </el-button>
          
          <el-button 
            class="flex-1!" 
            type="primary" 
            :disabled="disabled" 
            @click.stop="handleUseTemplate" 
          >
            Apply
          </el-button>
        </div>
      </component>
    </template>
  </MkResourceCard>
</template>

Summary of Actions:

  1. Rename handleUseTemplate to handleApplyTemplate for consistency.
  2. Verify that MkResourceCard supports the class and @click props. If it doesn't, wrap the card in a native <div> with those properties instead.
  3. Clarify UX: Decide if "Details" button is needed if the whole card is already clickable for details. If yes, keep it; if no, remove it to reduce clutter.
  4. Add optional chaining (template?.name) for safety.

@fit2cloudwxx
fit2cloudwxx merged commit c8272a8 into v3 Nov 10, 2026
3 of 4 checks passed
@fit2cloudwxx
fit2cloudwxx deleted the pr@v3@fix_templateCardDetailAndText branch November 10, 2026 06:43
<MkResourceCard :title="tool.name" class="cursor-pointer" @click="openDetailDrawer">
<template #icon>
<ToolIcon :icon="tool.icon" :size="24" :type="tool.tool_type" />
</template>

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 the potential issues and optimization suggestions for the provided code snippet:

1. Prop Validation Issue (function handleApply(tool = props.tool) {)

  • Irregularity: You are defining a function parameter tool with a default value referencing an external variable (props.tool) inside a script setup or composition context. If this is a plain function declaration inside <script> but outside a setup block, props might not be in scope, causing a ReferenceError. If it's inside setup, using props.tool as a default parameter in a standalone function definition is unconventional.
  • Suggestion: Define the function to explicitly rely on props.tool or pass arguments correctly from the template/method calls. If handleApply is meant to use the current tool, access props.tool directly inside the function body instead of using it as a parameter default unless necessary for reusability.

2. Event Handling & Performance (@click="openDetailDrawer" vs @click="handleApply")

  • Potential Issue: The diff changes the click handler from presumably nothing or something else to openDetailDrawer. However, the file name or context suggests handleApply is related to applying/using the tool. Ensure openDetailDrawer is the intended action. If handleApply was previously used, verify if behavior changed unintentionally.
  • Optimization: If openDetailDrawer doesn’t need event details (like $event), this is fine. If you plan to optimize future changes, consider using v-on="onClickHandlers" or similar patterns if multiple handlers are needed.

3. Accessibility (a11y)

  • Issue: A <template> component (MkResourceCard) likely renders a div or li by default. Adding a @click makes it interactive, but non-interactive HTML elements are not accessible to keyboard-only users (which are essential for screen readers).
  • Suggestion:
    • Either ensure MkResourceCard internally handles keyboard events (e.g., tabindex="0" + @keydown.enter).
    • Or add tabindex="0" and @keydown.enter="openDetailDrawer" manually if the component doesn’t support it.
    • Add role="button" and aria-label if semantic HTML isn’t used.

4. CSS Class Specificity (.cursor-pointer)

  • Observation: Hardcoding utility classes like cursor-pointer is common in Tailwind setups but can lead to inconsistency if other cards don’t have it. Consider defining a custom class .resource-card that includes cursor: pointer; if this style is universal for all resource cards.

5. Vue Version Compatibility

  • If using Vue 2, ensure that MkResourceCard emits or supports native DOM events properly. In Vue 3, fall-through attributes work seamlessly. Verify based on your project’s Vue version.

Recommended Improved Code Snippet:

<script setup>
// Example correction for handleApply if it's part of setup logic
// Instead of: function handleApply(tool = props.tool) { ... }
// Prefer explicit access:
const handleApply = () => {
  const toolToApply = props.tool; // Use reactive props directly
  // logic...
};

// Ensure openDetailDrawer is defined
const openDetailDrawer = () => {
  // drawer logic...
};
</script>

<template>
  <MkResourceCard 
    :title="tool.name" 
    class="cursor-pointer" 
    @click="openDetailDrawer"
    tabindex="0" 
    role="button" 
    :aria-label="'Open details for ' + tool.name"
    @keydown.enter.prevent="openDetailDrawer"
    @keydown.space.prevent="openDetailDrawer"
  >
    <template #icon>
      <ToolIcon :icon="tool.icon" :size="24" :type="tool.tool_type" />
    </template>
  </MkResourceCard>
</template>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants