Skip to content

stt #5633

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

Closed
DDMeaqua wants to merge 1 commit into from
Closed

stt #5633

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
2 changes: 1 addition & 1 deletion app/components/chat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1505,7 +1505,7 @@ function _Chat() {
setAttachImages(images);
}

// 快捷键 shortcut keys
// 快捷键
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Oct 11, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider potential shortcut conflicts and accessibility

The addition of keyboard shortcuts is a great feature for improving user experience. However, there are a few points to consider:

  1. Some of these shortcuts might conflict with browser or system shortcuts. For example, Cmd+Shift+C is often used for opening the developer console.
  2. The handleKeyDown function is quite long and handles multiple responsibilities. Consider refactoring it into smaller, more focused functions for better maintainability.
  3. Some shortcuts might not be intuitive for all users. Consider adding a way to customize shortcuts or at least document them clearly for users.

Consider refactoring the handleKeyDown function into smaller, more focused functions. For example:

const handleNewChat = (event: KeyboardEvent) => {
  if ((event.metaKey || event.ctrlKey) && event.shiftKey && event.key.toLowerCase() === "o") {
    event.preventDefault();
    chatStore.newSession();
    navigate(Path.Chat);
  }
};

const handleFocusInput = (event: KeyboardEvent) => {
  if (event.shiftKey && event.key.toLowerCase() === "escape") {
    event.preventDefault();
    inputRef.current?.focus();
  }
};

// ... other handler functions ...

useEffect(() => {
  const handleKeyDown = (event: KeyboardEvent) => {
    handleNewChat(event);
    handleFocusInput(event);
    // ... call other handler functions ...
  };

  window.addEventListener("keydown", handleKeyDown);
  return () => window.removeEventListener("keydown", handleKeyDown);
}, [messages, chatStore, navigate]);

This approach makes the code more modular and easier to maintain.

🛠️ Refactor suggestion

Improve robustness of DOM-dependent shortcuts

The shortcut for copying the last code block relies on selecting elements with the class "copy-code-button". This approach might be fragile if the DOM structure changes.

Consider using a more robust method to identify and interact with code blocks. For example, you could add data attributes to code blocks and use those for selection:

const copyLastCodeBlock = () => {
  const codeBlocks = document.querySelectorAll('[data-code-block]');
  if (codeBlocks.length > 0) {
    const lastCodeBlock = codeBlocks[codeBlocks.length - 1];
    const copyButton = lastCodeBlock.querySelector('[data-copy-button]');
    copyButton?.click();
  }
};

Then in your JSX where you render code blocks:

<div data-code-block>
  <pre>{/* code content */}</pre>
  <button data-copy-button onClick={handleCopy}>Copy</button>
</div>

This approach is less likely to break if class names or DOM structure change.

🛠️ Refactor suggestion

Enhance accessibility and user experience of ShortcutKeyModal

The addition of a ShortcutKeyModal is a great feature for user education. However, there are a few improvements that could be made:

  1. Consider adding keyboard navigation within the modal. Users should be able to tab through the shortcuts and close the modal using the keyboard.
  2. The shortcut to show the modal (Cmd+/ or Ctrl+/) is commonly used for commenting code in many IDEs. This might be confusing for some users. Consider allowing users to customize this shortcut or choosing a less commonly used combination.

To improve keyboard navigation, you could modify the ShortcutKeyModal component like this:

export function ShortcutKeyModal(props: { onClose: () => void }) {
  // ... existing code ...

  useEffect(() => {
    const handleKeyDown = (e: KeyboardEvent) => {
      if (e.key === 'Escape') {
        props.onClose();
      }
    };
    window.addEventListener('keydown', handleKeyDown);
    return () => window.removeEventListener('keydown', handleKeyDown);
  }, [props.onClose]);

  return (
    <div className="modal-mask">
      <Modal
        title={Locale.Chat.ShortcutKey.Title}
        onClose={props.onClose}
        actions={[
          <IconButton
            key="close"
            icon={<CloseIcon />}
            text={Locale.UI.Close}
            onClick={props.onClose}
            autoFocus // This will focus the close button when the modal opens
          />,
        ]}
      >
        {/* ... existing modal content ... */}
      </Modal>
    </div>
  );
}

This change allows users to close the modal with the Escape key and focuses the close button when the modal opens, improving keyboard accessibility.

const [showShortcutKeyModal, setShowShortcutKeyModal] = useState(false);

useEffect(() => {
Expand Down