Skip to content

fix: show/hide autoplay and autosend checkbox #2053

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
Jan 21, 2025
Merged

fix: show/hide autoplay and autosend checkbox #2053

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
4 changes: 2 additions & 2 deletions ui/src/views/application/ApplicationSetting.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@
</span>

<div class="flex">
<el-checkbox v-model="applicationForm.stt_autosend">{{
<el-checkbox v-if="applicationForm.stt_model_enable" v-model="applicationForm.stt_autosend">{{
$t('views.application.applicationForm.form.voiceInput.autoSend')
}}</el-checkbox>
<el-switch
Expand Down Expand Up @@ -331,7 +331,7 @@
>
</span>
<div class="flex">
<el-checkbox v-model="applicationForm.tts_autoplay">{{
<el-checkbox v-if="applicationForm.tts_model_enable" v-model="applicationForm.tts_autoplay">{{
$t('views.application.applicationForm.form.voicePlay.autoPlay')
}}</el-checkbox>
<el-switch
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Jan 21, 2025

Choose a reason for hiding this comment

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

The provided code appears to be from an application's Vue.js interface that manages form settings related to voice integration. After reviewing it, here are some suggestions:

  1. Conditional Rendering: All el-checkbox elements should ideally have their conditions checked before rendering. Here you have two instances where v-if="applicationForm.stt_model_enable" is missing and could lead to unnecessary renderings of checkboxes when the feature isn't enabled.

    <!-- Incorrect -->
<el-checkbox v-model="applicationForm.stt_autosend">{{ t('views.application.applicationForm.form.voiceInput.autoSend') }}<br />

// Correct with condition
< el-checkbox v-if="applicationForm.stt_model_enable" v-model="applicationForm.stt_autosend">{{ t('views.application.applicationForm.form.voiceInput.autoSend') }}

  2. Code Clarity: While not critical, consider adding comments around sections of code like conditional checks for better readability.

  3. Performance Considerations:

    • If applicationForm.stt_model_enable is expected to toggle frequently (e.g., due to server-side changes), ensure that reactive updates are optimized for performance since these might cause multiple re-renders.
    • Ensure all state change operations are minimal and efficient, especially in environments where resource management becomes crucial.

  4. TTS Configuration: Since the code handles both STT (Speech To Text) and TTS (Text To Speech) configurations, similar considerations apply to enabling/disabling those features through applicationForm.tts_model_enable.

Overall, these points improve the maintainability and efficiency of the code while ensuring that its functionality aligns with user expectations according to translations fetched via $t.

Expand Down
4 changes: 2 additions & 2 deletions ui/src/workflow/nodes/base-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
$t('views.application.applicationForm.form.voiceInput.label')
}}</span>
<div class="flex">
<el-checkbox v-model="form_data.stt_autosend">{{
<el-checkbox v-if="form_data.stt_model_enable" v-model="form_data.stt_autosend">{{
$t('views.application.applicationForm.form.voiceInput.autoSend')
}}</el-checkbox>
<el-switch
Expand All @@ -117,7 +117,7 @@
$t('views.application.applicationForm.form.voicePlay.label')
}}</span>
<div class="flex">
<el-checkbox v-model="form_data.tts_autoplay">{{
<el-checkbox v-if="form_data.tts_model_enable" v-model="form_data.tts_autoplay">{{
$t('views.application.applicationForm.form.voicePlay.autoPlay')
}}</el-checkbox>
<el-switch
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Jan 21, 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 appears to be part of an HTML template for a form within an application. It involves conditional rendering using Vue's v-if directive based on whether certain conditions (stt_model_enable, voice_input_enabled) are met before enabling checkboxes and switches related to voice input/output.

Here’s a summary of the checks and potential optimizations:

  1. Comments: The comments at lines 87–93 explain how various properties relate to different functionalities (voice input/input auto send/auto play). Ensure these explanations remain accurate if they need updating with new features or changes.

  2. Conditional Rendering:

    • At line 108, there seems to be additional logic that might not need to be displayed under some circumstances unless specific conditions dictate it.
    • This can be improved by ensuring that all relevant functionality is clearly documented alongside its respective UI elements.

  3. Language Translations:

    • Ensure that the translations used in $t(...) match those defined in your locale files or translation library. This helps avoid missing messages or incorrect display errors during runtime.

  4. Flexbox Usage:

    • Use Flexbox judiciously for layout purposes. Make sure each element (<el-checkbox> / <el-switch>) has appropriate width/height attributes to optimize responsiveness without unnecessary padding/margins.

Code Suggestion

<div class="input-options flex-column m-y-4">
  <!-- Voice Input -->
  <span>{{ $t('views.application.applicationForm.form.voiceInput.label') }}</span>
  <el-checkbox v-model="form_data.vic_input_enabled">{{ 
    // Display "autoSend" only when stt_model_enable = true
    $t(form_data.vic_input_enabled ? 'views.application.applicationForm.form.voiceInput.autoSend' : '') 
  }}</el-checkbox>

  <!-- Toggle for TTS Auto Play -->
  <span>{{ $t('views.application.applicationForm.form.voicePlay.label') }}</span>
  <el-switch v-model="form_data.vic_automode_play"
             :disabled="{true: !vic_tts_model_enabled || vic_voiceenabled}"
             size="mini"></el-switch>
</div>

Summary of Key Observations and Recommendations:

  • Documentation Updates: Clarify any assumptions made in the existing comments about what each component relates to.

  • Logical Checks: Ensure logical flow between components is clear; e.g., don't unnecessarily use ternary operators that could confuse developers later.

  • Responsive Layouts: Utilize CSS utility classes such as Bootstrap or TailwindCSS for consistent styling across devices.

By addressing these points, you can ensure the code remains readable and maintainable while also improving user experience through optimal layout design and effective language integration.

Expand Down
Loading