-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Conversation
…+enter for new line
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. |
[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 |
allow="microphone"> | ||
</iframe> | ||
` | ||
|
||
dialogVisible.value = true | ||
} | ||
|
There was a problem hiding this comment.
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:
-
Variable Reusability: The
computed
propertiessource1
,urlParams1
and similar should ideally be reused instead of being computed multiple times with the same logic. -
Code Duplication: There is some duplication between setting up different iframe sources (
source1
,source2
,source3
). This can be refactored to avoid redundancy. -
Security Considerations: Ensure that user inputs like
val
in the iframe URLs do not contain harmful characters that could lead to injection vulnerabilities. -
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. -
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, |
There was a problem hiding this comment.
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:
-
Code Block Formatting: There's an extra space before the
</script>
tag. Remove this space to ensure proper formatting. -
nextTick()
Usage: In theinsertNewlineAtCursor
function, there is no need fornextTick()
because it already uses DOM manipulation to handle selection after inserting text. -
Variable Declarations: Ensure that all variables are properly initialized and used correctly within their scopes. For example, make sure
inputValue
,isDisabledChart
, andloading
are defined elsewhere in your component. -
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 `//
|
||
const showUserInputContent = computed(() => { | ||
return ((isUserInput.value && firsUserInput.value) || showUserInput.value) && props.type !== 'log' | ||
}) | ||
watch( | ||
() => props.chatId, | ||
(val) => { |
There was a problem hiding this comment.
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:
-
Computed Property Optimization:
- The
showUserInputContent
computed property is redundant because it effectively does the same thing asshowUserInput
. You can remove one of them to simplify the logic.
- The
-
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.
- Variables like
-
Template Syntax Simplification:
- Some unnecessary
<template>
tags can be removed. For example, the innermost template tag can often be simplified.
- Some unnecessary
-
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.
- Ensure that all conditions in the template (
-
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.
…+enter for new line
What this PR does / why we need it?
Summary of your change
Please indicate you've done the following: