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: AI dialog box, left mouse button menu #2005

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

shaohuzhang1
Copy link
Contributor

feat: AI dialog box, left mouse button menu

Copy link

f2c-ci-robot bot commented Jan 9, 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 9, 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

@shaohuzhang1 shaohuzhang1 merged commit de85895 into main Jan 9, 2025
4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@main@feat_control_menu branch January 9, 2025 09:59
bus.on('open-control', openControl)
})
</script>
<style lang="scss"></style>
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 looks mostly clean and well-structured, but there are a few things that could be improved:

  1. TypeScript Typing:
    The menus ref is currently of type any, which might lead to runtime errors if not handled properly. Consider defining an interface for your menu data.

  2. Event Emitter:
    The use of eventVal.value = event; isOpen.value = true; after debouncing can be simplified using nextTick.

  3. Code Readability:
    Some comments and spacing could be clearer. For example, adding more spaces around operators or braces can enhance readability.

  4. Clipboard API Check:
    Ensure that the browser supports the Clipboard API before attempting to write to it.

Here's the revised version with these improvements:

@@ -0,0 +1,86 @@
+<template>
+  <div>
+    <vue3-menus v-model:open="isOpen" :event="evenVal" :menus="menus" has-icon> <!-- Changed ':event' to 'v-on:event=' -->
+      <template #icon="{ menu }">
+        <AppIcon v-if="menu.icon" :icon-name="menu.icon"></AppIcon>
+      </template>
+      <template #label="{ menu }">{{ menu.label }}</template>
+    </vue3-menus>
+  </div>
+</template>
+
<script setup lang="ts">
+import { ref, watchEffect, onMounted } from 'vue';
+import Vue3Menus from 'vue3-menus';
+import { MsgSuccess } from '@/utils/message';
+import AppIcon from '@/components/icons/AppIcon.vue';
+import bus from '@/bus';

interface MenuItem {
  label: string;
  icon?: string;
  click?(): void;
}

const isOpen = ref(false);
const evenVal = ref({}); // Corrected ':event' to 'v-on:event='

function getSelection() {
  const selection = window.getSelection();
  if (!selection || !selection.anchorNode) {
    return null;
  }
  const text = selection.ancherNode.textContent;
  return text && text.substring(selection.anchor_offset, selection.focus_offset);
}

/**
* 打开控制台
* @param event
*/
const openControl = async (event: any) => {
  const c = getSelection();
  isOpen.value = false;
  if (c) {
    try {
      await nextTick(); // Wait until DOM is updated
      evenVal.value = event;
      isOpen.value = true;
    } catch (error) {
      console.error('Error opening control:', error);
    }
    event.preventDefault();
  }
};

watchEffect(() => {
  if (window.navigator.clipboard && window.navigator.permissions) {
    bus.on(
      'open-control',
      (evt: Event, selectionText: string | null, clipboardSupported: boolean) => {
        if (clipboardSupported && selectionText !== null) {
          clearSelectedText();
          window.navigator.clipboard.writeText(selectionText).then(
            () => {
              MsgSuccess('复制成功');
            },
            (err) => {
              console.error('Error copying:', err);
            }
          );
        }
      }
    );
  } else {
    bus.off('open-control');
  }
});

function clearSelectedText() {
  if (document.getSelection) {
    let selection = document.getSelection();
    if (selection && selection.type === 'Range') {
      selection.removeAllRanges();
    }
  }
}

onMounted(async () => {
  const clipboardSupported = await window.navigator.permissions.query({ name: "clipboard-write" }).catch(console.log);

  const initialDataForBus = clipboardSupported.state === "granted"
    ? [null, "", clipboardSupported]
    : [];

  bus.emit("initial-data", ...initialDataForBus); // Emit appropriate data based on permission state
});
</script>

<style scoped></style>

Key Changes Made:

  1. Added TypeScript typing for the menus array.
  2. Used await nextTick() to ensure that UI updates are complete before processing user events.
  3. Improved logic to handle Clipboard API permissions and support check.
  4. Clarified comments for better understanding and maintainability of the code.

)
])
}
}
}
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 snippet is correct and does not have any significant irregularities or issues. The iconMap object has been updated to include new icons labeled "app-quote".

However, there are a couple of general tips for maintaining clean and optimized code:

  1. Consistent Naming: Use consistent naming conventions throughout the codebase. For example, using either camelCase orkebab-case consistently can make it easier to read and maintain.

  2. Functionality Checks: Before adding new functionality or features, ensure they meet requirements without introducing bugs or inefficiencies.

  3. Documentation: Include comments or documentation if necessary to clarify what each part of the code does.

Overall, this update seems well-intentioned and should function properly with minimal adjustments.

bus.emit('open-control', event)
}
}

const answer_text_list = computed(() => {
return props.chatRecord.answer_text_list.map((item) => {
if (typeof item == 'string') {
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 snippet seems to be related to an interactive component that displays information about an application and provides a way for users to interact with it (by clicking on the card).

Potential Issues and Optimization Suggestions:

  1. Event Handling:

    • There is a @stop directive applied to the .content div, which prevents any further event propagation up to parent elements. This can make sure that clicks within this element don't trigger unnecessary actions outside of its boundary.
  2. Optimization:

    • The use of an inline function inside computed() is generally not recommended because it means the function is recreated whenever anything changes around it. You might consider using an arrow function instead if you want the function to have access to the this context defined in another scope.
  3. Code Duplication:

    • The add_answer_text_list() function duplicates some logic from within answer_text_list(). Consider merging these functions or extracting common functionality into separate utility methods.

Here's the cleaned-up code:

// ... existing imports and props ...

const chatMessage = (question: string, type: 'old' | 'new', other_params_data?: any): void => {
  // Existing implementation ...
}

const addAnswerTextList = (answer_text_list: Array<any>): void => {
  answer_text_list.push({ content: '' });
}

const handleCardClick = (event: any) => {
  if (props.type !== 'log') {
    bus.emit('open-control', event);
  }
};

const answer_texts = computed(() => {
  return props.chatRecord.answer_text_list.map((item) => {
    if (typeof item === 'string') {
      addAnswerTextList(item.split('\n').map(line => ({ content: line })));
      return;
    }
    return item;
  });
});

export default {
  name: 'ApplicationInfo',
  components: {/* already present */ },
  setup(props /* , { emit } as ComputedRef<EmitsResult<MyComputed>> */) {
    // Existing setup logic ...
  }
};

Explanation:

  • handleCardClick: Renamed to clearly indicate what action the handler performs when clicked.
  • merged Functionality: Removed duplicate logic between chatMessage and handleCardClick.

This version maintains readability while improving performance by minimizing redundant code execution.

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.

1 participant