这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@timothycarambat
Copy link
Member

resolves #286

@timothycarambat timothycarambat merged commit 9f1a5d9 into master Oct 25, 2023
@timothycarambat timothycarambat deleted the 286-add-back-query-chat-mode-sel branch October 25, 2023 23:01
@review-agent-prime
Copy link

The code changes in this PR look good overall. However, I have a few suggestions to improve the code quality and maintainability.

  1. In the ChatModeSelector function, you are using window.localStorage directly. It would be better to abstract this into a custom hook or utility function. This would make the code more testable and maintainable.
// utils/storage.js
export function useLocalStorage(key, initialValue) {
  const [storedValue, setStoredValue] = useState(() => {
    try {
      const item = window.localStorage.getItem(key);
      return item ? JSON.parse(item) : initialValue;
    } catch (error) {
      console.log(error);
      return initialValue;
    }
  });

  const setValue = value => {
    try {
      const valueToStore = value instanceof Function ? value(storedValue) : value;
      setStoredValue(valueToStore);
      window.localStorage.setItem(key, JSON.stringify(valueToStore));
    } catch (error) {
      console.log(error);
    }
  };

  return [storedValue, setValue];
}

// In your component
const [chatMode, setChatMode] = useLocalStorage(STORAGE_KEY, "chat");
  1. The tooltip delay is hardcoded as 700. It would be better to make this a constant at the top of the file or even better, a prop with a default value. This would make it easier to adjust the delay in the future.
const TOOLTIP_DELAY = 700;

// In your function
setTimeout(() => {
  setShowTooltip(true);
}, TOOLTIP_DELAY);
  1. The tooltip text is hardcoded in the component. It would be better to move this to a function that generates the tooltip text based on the current chat mode. This would make the code more maintainable and easier to change in the future.
function getTooltipText(chatMode) {
  return `You are currently in ${chatMode} mode. Click to switch to ${
    chatMode === "chat" ? "query" : "chat"
  } mode.`;
}

// In your component
<div className={`opacity-${showToolTip ? 1 : 0} ...`}>
  {getTooltipText(chatMode)}
</div>
  1. The cleanupTooltipListener function is defined inside the ChatModeSelector function. It would be better to move this function outside of the ChatModeSelector function to avoid redefining it on every render.
function cleanupTooltipListener(delayHandler, setShowTooltip) {
  clearTimeout(delayHandler);
  setShowTooltip(false);
}

// In your component
onMouseLeave={() => cleanupTooltipListener(delayHandler, setShowTooltip)}

Please consider these suggestions to improve the code quality and maintainability.

cabwds pushed a commit to cabwds/anything-llm that referenced this pull request Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add back in Query/Chat support in the input modal

2 participants