Skip to content
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.

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

feat: Support stt mode auto send message #1977

Merged
merged 1 commit into from
Jan 6, 2025
Merged

Conversation

shaohuzhang1
Copy link
Contributor

feat: Support stt mode auto send message --story=1017616 --user=刘瑞斌 【应用】语音输入支持自动发送 https://www.tapd.cn/57709429/s/1641715

--story=1017616 --user=刘瑞斌 【应用】语音输入支持自动发送 https://www.tapd.cn/57709429/s/1641715
Copy link

f2c-ci-robot bot commented Jan 6, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

f2c-ci-robot bot commented Jan 6, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

uploadAudioList.value = []
uploadVideoList.value = []
quickInputRef.value.textareaStyle.height = '45px'
autoSendMessage()
}
}
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code seems to be a Vue.js component that handles file uploads, real-time recording of voice messages, and sending text-based messages with attachments. Here are some observations, potential issues, and optimization suggestions:

Observations:

  1. Import Statements: The nextTick utility is used multiple times, which can optimize rendering by ensuring that DOM updates occur after certain operations have been completed.
  2. Auto-Message Sending: The autoSendMessage function is called when the user hits the Enter key (after disabling the default behavior), but it could benefit from more robust logic, such as checking if the message is not empty before proceeding.
  3. Input Value Resetting: After uploading messages or files, there's no explicit reset strategy for input fields like inputValue, uploadImageList, etc., especially during automated sends. This might lead to unexpected state management.
  4. Template Structure: The template structure looks correct, but ensure all components (RecordPlayer.vue, quick-input-ref) are properly imported and referenced.

Potential Issues:

  1. Data Management: There's no clear separation between data states (e.g., inputValue, upload*List) and computed properties, which can make it harder to debug and maintain.
  2. Error Handling: Although error handling exists, the responses should include meaningful feedback to users, possibly using the existing MsgAlert utility.
  3. Performance Optimization: Ensure that critical operations do not block the main thread (such as API calls, DOM updates) unnecessarily.

Optimization Suggestions:

  1. State Reconciliation: Implement a consistent approach for resetting UI elements after actions like message sending. For example, use Vuex or another state management library to keep track of form inputs and output lists.
  2. Lazy Load Components: If RecordPlayer.vue is complex or heavy, consider lazy loading it only when needed.
  3. Improve Feedback Mechanisms: Enhance the messaging system to provide immediate visual feedback (like success animations) or toast notifications about successful actions.
  4. Code Refactoring: Consider breaking down functions into smaller, more manageable pieces, adhering to principles like Single Responsibility Principle (SRP).
  5. Type Safety: While TypeScript types seem present, ensure they cover all necessary scenarios where errors can occur (e.g., incorrect file formats).

Overall, the code appears functional and complete, but refining its structure, performance, and clarity will enhance its sustainability and reliability. Let me know if you need further assistance!

size="small"
v-model="applicationForm.stt_model_enable"
@change="sttModelEnableChange"
/>
</div>
</template>
<el-select
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code has several issues:

  1. Duplicate el-tooltip component: There seems to be an issue with the <el-tooltip> component where it appears multiple times within the same template slot.

  2. Unnecessary spacing: There is extra space around certain elements, which could affect layout readability.

  3. Inconsistent formatting: The HTML structure and CSS classes are inconsistent throughout the file. It's important to maintain a consistent style for better readability and maintainability.

  4. Unused import statement: There is an unused import import AppIcon from '@/components/comm/AppIcon.vue'; at the top of the file. This should be removed since it's not being used anywhere.

Here’s a suggested optimized version of the code:

<template>
  <el-form-item :label-width="formLabelWidth">
    <div class="flex-between">
      <span class="mr-4">语音输入</span>
      <div class="flex">
        <el-checkbox v-model="applicationForm.stt_autosend">自动发送</el-checkbox>
        <el-switch 
          class="ml-8"
          size="small"
          v-model="applicationForm.stt_model_enable"
          @change="sttModelEnableChange"
        ></el-switch>
      </div>
    </div>
    <!-- Other form-items... -->
  </el-form-item>
</template>

<script>
export default {
  // ...
};
</script>

<style scoped>
.flex-between {
  display: flex;
  justify-content: space-between;
}

.el-checkbox {
  margin-right: 8px; /* Adjusted margin */
}
</style>

Optimization Suggested Points:

  • Consistent Class Names: Use the .flex-between class consistently across similar items.
  • Remove Unused Imports: Review imports and ensure all components are correctly imported and used.
  • Use Template Slots Properly: If necessary, move tooltip logic outside the item label if it doesn't fit well there.
  • Adjust Styles: Ensure styles like padding, margins, and font sizes are appropriate for your design requirements.

By applying these changes, the code becomes more uniform, easier to read, and maintains consistency in its styling.

size="small"
v-model="form_data.stt_model_enable"
@change="sttModelEnableChange"
/>
</div>
</template>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code you provided has a few issues:

  1. Redundant <template #label> Content: The #label template slot now includes the auto-send checkbox without any separators or explanations.

  2. Conflicting Properties: There appears to be a mix-up with properties (form_data.stt_autosend and v-model="form_data.stt_model_enable"). These should either use one of them or separate them appropriately.

  3. Improper Structure: The checkboxes and switches may not behave as expected due to the lack of proper structure within their containers.

Here's an optimized version of the code:

<template>
  <el-form-item>
    <template #label>
      <div class="flex-between">
        <p>
          <span class="mr-4">语音输入</span>
          <!-- Add description here if needed -->
        </p>

        <div class="flex align-center">
          <el-checkbox v-model="form_data.stt_autosend">{{ $t('common.auto_send') }}</el-checkbox>
          
          <el-switch
            class="ml-8"
            style="--el-slider-button-size: 0;"
            :active-text="$t('settings.enable_stt')"
            size="small"
            v-model="form_data.stt_model_enable"
            @change="sttModelEnableChange"
          />
        </div>

      </div>
    </template>
    <!-- Rest of form item elements -->
  </el-form-item>
</template>

<script>
export default {
  data() {
    return {
      form_data: { ... } // Initialize your form_data object here
    };
  },
  methods: {
    sttModelEnableChange(enabled) {
      this.$emit('update:model-value', { ...this.form_data, stt_enabled: enabled });
      this.handleSttInput(enabled);
    },
    handleSttInput(autostart) {
      // Implement logic to enable/disable automatic start based on autostart value
    }
  }
};
</script>

Key Changes:

  • Simplified label template for better readability and maintainability.
  • Removed redundant switch element.
  • Added tooltips using i18n for translations.
  • Ensured consistent spacing and alignment in the label area.
  • Used appropriate HTML tags and attributes to improve SEO performance.

These changes should resolve most issues while maintaining clear presentation and functionality. Make sure to adjust the labels and placeholder text according to your application requirements.

@liuruibin liuruibin merged commit c307f6b into main Jan 6, 2025
4 of 5 checks passed
@liuruibin liuruibin deleted the pr@main@feat_stt_audo_send branch January 6, 2025 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants