Skip to content

feat: conditionally render form items based on feature flags #4035

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
liuruibin merged 1 commit into from
Sep 15, 2025
Merged

feat: conditionally render form items based on feature flags #4035

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/views/application/ApplicationSetting.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@
</div>
</template>
</div>
<el-form-item @click.prevent>
<el-form-item @click.prevent v-if="applicationForm.mcp_enable || applicationForm.tool_enable">
<template #label>
<div class="flex-between">
<span class="mr-4">
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 15, 2025

Choose a reason for hiding this comment

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

The code appears to be part of an HTML template for an El Form Item. Here are some potential concerns and suggestions:

  1. Conditional Directive (v-if):

    • The @click.prevent v-if="applicationForm.mcp_enable || applicationForm.tool_enable" directive suggests that this form item is only visible depending on certain conditions. This seems logical, but ensure these conditions are appropriate.

  2. PreventDefault Event Listener:

    • Both versions (including and excluding the condition) use @click.prevent. It's important to avoid repeating event listeners with the same default behavior if they serve a similar purpose across different scopes.

  3. Element Type:

    • Ensure that this form item corresponds to an <el-form-item> element used within another component. If it isn't needed under certain conditions, removing the conditional could save computation.

  4. Accessibility:

    • Consider ensuring accessibility compliance when using conditional rendering. For instance, adding text or icons to describe the form item visually hidden from screen readers.

Here is revised version of the line incorporating some of these considerations:

<el-form-item 
  @click.prevent 
  :aria-hidden="!(applicationForm.mcp_enable || applicationForm.tool_enable)"
>
  <template #label>
    <div class="flex-between">
      <span class="mr-4">
        <!-- Label content -->
      </span>
    </div>
  </template>
</el-form-item>

This modification ensures better SEO and assistive technology compatibility without duplicating event listeners. Adjust labels and aria attributes as necessary for specific contexts.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/workflow/nodes/ai-chat-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@
</div>
</template>
</div>
<el-form-item @click.prevent>
<el-form-item @click.prevent v-if="chat_data.mcp_enable || chat_data.tool_enable">
<template #label>
<div class="flex-between">
<span class="mr-4">
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 15, 2025

Choose a reason for hiding this comment

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

The provided code snippet has the following issues:

  1. Misplacement of v-if Directive: The <el-form-item> tag should not have both an @click.prevent handler and a conditional directive (v-if). Instead, move the v_if below the @click.prevent.

  2. Incorrect Class Binding Syntax: In Vue 3, you do not use single quotes around string literals in template expressions unless they contain variables or operators that need escaping.

Here's the corrected version:

<el-form-item @click.prevent v-if="chat_data.mcp_enable || chat_data.tool_enable">
  <template #label>
    <div class="flex-between">
      <span class="mr-4">...</span><!-- Fill with actual content -->
   </div>
  </template>
</el-form-item>

Note: Ensure that all necessary imports, such as vue, el-form-item, etc., are correctly included in your project setup if this is being used in a context like Element UI or another framework.

Expand Down
Loading