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: Add mobile mode to the conversation, support ctrl/shift/cmd/opt… #2615

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

wangdan-fit2cloud
Copy link
Contributor

…+enter for new line

What this PR does / why we need it?

Summary of your change

Please indicate you've done the following:

  • Made sure tests are passing and test coverage is added if needed.
  • Made sure commit message follow the rule of Conventional Commits specification.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed.

Copy link

f2c-ci-robot bot commented Mar 19, 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 Mar 19, 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

allow="microphone">
</iframe>
`

dialogVisible.value = true
}

Copy link
Contributor

Choose a reason for hiding this comment

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

There are a few areas where the code can be improved:

  1. Variable Reusability: The computed properties source1,urlParams1 and similar should ideally be reused instead of being computed multiple times with the same logic.

  2. Code Duplication: There is some duplication between setting up different iframe sources (source1, source2, source3). This can be refactored to avoid redundancy.

  3. Security Considerations: Ensure that user inputs like val in the iframe URLs do not contain harmful characters that could lead to injection vulnerabilities.

  4. Performance Optimization: Using vue-apollo's .watch method might trigger more frequently than necessary. If you only need this behavior under certain conditions, consider optimizing accordingly.

  5. Inline Styles vs. Classes: Inline styles are generally less flexible than using classes due to how JavaScript interacts with them directly; however, your current implementation uses both methods which is fine.

Here's an updated version assuming you want to streamline it while keeping functionality intact:

<script setup lang="ts">

import { onMounted, ref } from 'vue';
import AppIcon from '@/components/AppIcon.vue'

let dialogVisible = ref(false)

// Store initial states as array
const sources = [source1, source2, source3]

for(let i=0 ; i<sources.length ; ++i){
    sources[i] = ref('');
}

const modeOptions = ["fullscreen", "mobile", "floating"];
const apiInputParams = ref(''); // Use this variable

onMounted(() => {
    setSource(1); // initialize first mode
})

function copyClick(value: string): void {
    navigator.clipboard.writeText(value);
}

function setSource(modeIndex: number): void {
    let targetURL = application.location;
    
    targets[modeIndex].value = `
    <button onclick="openPopup('${targetURL}',${params})">Try It</button>
    <script type='text/template'>
    function openPopup(url,val){ window.open(`${window.location.origin}/api/application/embed/${mode}`,
        `${window.location.hostname}:${window.location.port}`)
    }
    </script>`
    
    switch (modeIndex) {
        case 0:
            break;
        case 1:
            targetURL = targetURL + val;
            addTargetValue(src="${targetURL}${params}",height = '400',width = '400'), style = 'all');
            break;
        default:
            addTargetValue(`<iframe src="${targetURL}${params}" style="width:${modeWidth}px;height:${modeHeight}px;"></iframe>`)
    }

    dialogVisible.value = true;
}        

function addTargetValue(htmlContent: string, height?: number | string, width?: number | string, cssClass?: string): void{
    modesHtml.append(htmlContent);

    const frameElement = document.createElement("iframe");
    if(height && typeof height === 'number'){
        frameElement.style.height = height.toString();
    }else if(width && typeof width === 'string'){
        frameElement.style.width = width;
    }

    if(cssClass){
        frameElement.className = cssClass
    } else if(window.innerWidth > 600){
        frameElement.src = window.document.baseURI+ "/assets/frame-full-width.html?url="+ encodeURIComponent(application.location.replace("https://github.com/","").replace(/\/$/, ""));
    }else{
        frameElement.src = window.document.baseURI+"/assets/frame-mobile.html?url="+encodeURIComponent(application.location.replace("https://github.com/",""));
    }
}

This snippet assumes that modesHtml element has already been created somewhere within template part of Vue component where it would appear on screen when dialogVisible becomes true. Adjust according to actual structure of your Vue Template section properly!

textarea.setSelectionRange(startPos + 1, startPos + 1) // 光标定位到换行后位置
})
}

function getHitTestList() {
const obj = {
query_text: inputValue.value,
Copy link
Contributor

Choose a reason for hiding this comment

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

The provided code has a few minor improvements and changes:

  1. Code Block Formatting: There's an extra space before the </script> tag. Remove this space to ensure proper formatting.

  2. nextTick() Usage: In the insertNewlineAtCursor function, there is no need for nextTick() because it already uses DOM manipulation to handle selection after inserting text.

  3. Variable Declarations: Ensure that all variables are properly initialized and used correctly within their scopes. For example, make sure inputValue, isDisabledChart, and loading are defined elsewhere in your component.

  4. Logical Flow: The condition logic for handling Ctrl/Shift/Cmd/Opt+Enter is mostly correct, but you may want to add additional checks or conditions based on specific use cases.

Here's the cleaned-up version of the JavaScript part:

<template>
 <!-- Your template content here -->
</template>

<script setup lang="ts">
import { reactive, ref, onMounted, computed } from 'vue'
import { useRoute } from 'vue-router'
import { cloneDeep } from 'lodash'
import datasetApi from '@/api/dataset'

function editParagraph(row: any) {
}

function sendChatHandle(event: any) {
  if (!(event ?? {}).ctrlKey && !(event ?? {}).shiftKey && !(event ?? {}).altKey && !(event ?? {}).metaKey) {
    event.preventDefault()
    if (!isDisabledChart.value && !loading.value) {
      getHitTestList()
    }
  } else {
    insertNewlineAtCursor(event)
  }
}

const insertNewlineAtCursor = (event?: any): void => {
  const textarea = document.querySelector('.el-textarea__inner') as HTMLTextAreaElement | null; // Optional chaining
  if (!textarea) return;
  
  const startPos = textarea.selectionStart || 0; // Default start position if selection starts at zero
  let endPos = textarea.selectionEnd || startPos;

  event.preventDefault();
  userInput.value = `${userInput.value.slice(0, startPos)}\n${userInput.value.slice(endPos)}`;

  textarea.setSelectionRange(startPos + 1, startPos + 1);
};

function getHitTestList() {
  const obj = {
    query_text: userInput.value,
    //
  };
  // ...
}
</script>

This should resolve most of the identified issues. Make sure to replace placeholders like `//

@wangdan-fit2cloud wangdan-fit2cloud merged commit 7b51d08 into main Mar 19, 2025
4 checks passed
@wangdan-fit2cloud wangdan-fit2cloud deleted the pr@main/mobile-mode branch March 19, 2025 07:31

const showUserInputContent = computed(() => {
return ((isUserInput.value && firsUserInput.value) || showUserInput.value) && props.type !== 'log'
})
watch(
() => props.chatId,
(val) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The code snippet you provided has several areas that need attention:

  1. Computed Property Optimization:

    • The showUserInputContent computed property is redundant because it effectively does the same thing as showUserInput. You can remove one of them to simplify the logic.
  2. Variable Redundancy:

    • Variables like _inputVisible, _inputShowed, and _focusStatus are declared but not used. These should be removed or reassigned if they are meant to have a purpose.
  3. Template Syntax Simplification:

    • Some unnecessary <template> tags can be removed. For example, the innermost template tag can often be simplified.
  4. Conditional Rendering:

    • Ensure that all conditions in the template (v-if) and computed properties align. Sometimes, conditions might lead to unexpected behavior due to logical errors.
  5. Code Consistency:

    • There's some inconsistency in variable naming conventions and typescript-specific syntax (like using Vue-specific decorators without imports). Ensure consistency across the codebase.

Here’s an optimized version of the code considering these points:

<template>
  <div ref="aiChatRef" class="ai-chat" :class="type">
    <div
      v-show="(firsUserInput && type !== 'log') || showUserInput"
      :class="firsUserInput ? 'firstUserInput' : 'popperUserInput'"
    >
      <UserForm
        v-if="$slots.user-form"
        ref="userFormRef"
      ></UserForm>
    </div>
    <el-scrollbar ref="scrollDiv" @scroll="handleScrollTop">
      <div ref="dialogScrollbar" class="ai-chat__content p-24">
        <PrologueContent />
        <!-- Additional content -->
      </div>
    </el-scrollbar>
    <div v-if="!isUserInput || !firsUserInput">
      <button 
        v-if="props.applicationDetails.work_flow?.nodes?.filter((v: any) => v.id === 'base-node')[0].properties.user_input_field_list.length > 0" 
        class="user-input-button mb-8" 
        type="primary" 
        text 
        @click="toggleUserInput"
      >
        <AppIcon iconName="app-user-input"></AppIcon>
      </button>      
    </div>
  </div>
</template>

<script setup lang="ts">
import { ref, computed, watch } from "vue";
import UserForm from "@/components/UserForm.vue";
import PrologueContent from "@/components/PrologueContent.vue";
import AppIcon from "@/components/AppIcon.vue";

interface Props {
  applicationDetails: Record<string, any>;
  chatId: string;
  // other props ...
}

defineProps<Props>();

const emit = defineEmits(["refresh", "scroll"]);

// Store usage
const { application, common } = useStore();

const isMobile = computed(() => {
  return common.isMobile() || mode === "embed";
});

const aiChatRef = ref();
const scrollDiv = ref();

const isUserInput = computed(() =>
  Boolean(
    props.applicationDetails.work_flow?
      .nodes?.filter((v: any) => v.id === "base-node")[0]?
      .properties?.user_input_field_list?.length
  ) ||
  false
);

watch(() => props.chatId, (val) => {});
</script>

Key Changes:

  • Removed Unused Properties: Removed variables like _inputVisible, _inputShowed, and _focusStatus.
  • Simplified Template: Removed unnecessary <template> tags.
  • Consistent Logic: Ensured consistent conditionals throughout the component to avoid logical discrepancies.
  • Typo Correction: Corrected the typo in mode to "embed" instead of "emibe".

This cleaned-up version reduces redundancy, maintains readability, and ensures the component behaves predictably. Make sure all changes align with your project's requirements and best practices.

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