clickhouse-docs icon indicating copy to clipboard operation
clickhouse-docs copied to clipboard

Improvement: auto focus search

Open Blargian opened this issue 9 months ago • 7 comments

Issue:

When using the Algolia search popup, the search bar does not focus automatically.

Desired behaviour:

the search input should automatically focus when the pop up opens.

steps to reproduce:

  • Open the ClickHouse docs page using a new Chrome tab (https://clickhouse.com/docs)
  • Press CMD+K (this opens the Algolia search popup). However, the text input box in the popup is not automatically focused.
  • Manually click on the text input box.
  • Type in a string to filter the content.

Blargian avatar Feb 16 '25 23:02 Blargian

I would love to help solve that issue. Would you like assigning it to me?

gneo0 avatar Feb 17 '25 22:02 gneo0

I've been investigating this issue and noticed that the text input of Algolia search popup focuses correctly. The problem of this happens when the user presses Ctrl + K on the initial render, the Algolia search popup is being rendered three times instead of once. However, on initial render clicking inside the search input once to trigger Algolia popup, after closing the Algolia search popup and then pressing Ctrl + K works correctly and renders the Algolia search popup only once as expected.

How to reproduce that:

  1. On initial render of Clickhouse docs, press Ctrl + K.
  2. Type some text on your keyboard.
  3. Then, click two times on background div element of Algolia popup like you are going to close it.
  4. Now, you must be able to see the text that you had typed.

I suspect that the event listener for Ctrl + K might be attached multiple times, but I haven't been able to locate where this event is handled in the codebase.

If anyone is familiar with how the Algolia search popup is triggered, could you point me to the relevant file/component? Any help would be greatly appreciated!

Thanks in advance!

gneo0 avatar Feb 18 '25 16:02 gneo0

Hey @gneo0, appreciate you picking this up! The handler for the search is here I believe:

https://github.com/ClickHouse/clickhouse-docs/blob/608e46475c26e32175ebac2ce8094173f497f27a/src/theme/SearchBar/index.js#L22-L33

It's also used here for the knowledge base page search, but without an event handler.

https://github.com/ClickHouse/clickhouse-docs/blob/608e46475c26e32175ebac2ce8094173f497f27a/src/theme/BlogSidebar/Desktop/index.js#L34

Blargian avatar Feb 18 '25 20:02 Blargian

Hey @Blargian , the bug doesn't appear when user clicks on search bar but on Ctrl + K keydown event happens and only at the initial render of the clickhouse docs website. The problem is that I can't locate where on the code this keydown event happens. Thanks for your provided answer and I'm looking forward if you could help me further where Ctrl + K event happens!

gneo0 avatar Feb 19 '25 12:02 gneo0

@gneo0 Apologies, you're right. The event handler itself looks like its defined as part of docsearch. I found it as minified JS only by searching through node_modules for "cmd + k". Unminified reads as:

function useDocSearchKeyboardEvents({
  isOpen,
  onOpen,
  onClose,
  onInput,
  searchButtonRef,
}: { // UseDocSearchKeyboardEventsProps - this type was missing
  isOpen: boolean;
  onOpen: () => void;
  onClose: () => void;
  onInput: (event: KeyboardEvent) => void;
  searchButtonRef: React.RefObject<HTMLButtonElement>;
}): void {
  React.useEffect(() => {
    function onKeyDown(event: KeyboardEvent): void {
      const isEditingContent = (event: KeyboardEvent): boolean => {
        // Check if the user is currently editing content in an input, textarea, or select element.
        const activeElement = document.activeElement;
        if (activeElement instanceof HTMLInputElement ||
            activeElement instanceof HTMLTextAreaElement ||
            activeElement instanceof HTMLSelectElement) {
            return true;
        }
        return false;
      };

      if (
        (event.code === 'Escape' && isOpen) ||
        // The `Cmd+K` shortcut both opens and closes the modal.
        // We need to check for `event.key` because it can be `undefined` with
        // Chrome's autofill feature.
        // See https://github.com/paperjs/paper.js/issues/1398
        (event.key?.toLowerCase() === 'k' && (event.metaKey || event.ctrlKey)) ||
        // The `/` shortcut opens but doesn't close the modal because it's
        // a character.
        (!isEditingContent(event) && event.key === '/' && !isOpen)
      ) {
        event.preventDefault();

        if (isOpen) {
          onClose();
        } else if (!document.body.classList.contains('DocSearch--active')) {
          // We check that no other DocSearch modal is showing before opening
          // another one.
          onOpen();
        }

        return;
      }

      if (searchButtonRef && searchButtonRef.current === document.activeElement && onInput) {
        if (/[a-zA-Z0-9]/.test(String.fromCharCode(event.keyCode))) {
          onInput(event);
        }
      }
    }

    window.addEventListener('keydown', onKeyDown);

    return (): void => {
      window.removeEventListener('keydown', onKeyDown);
    };
  }, [isOpen, onOpen, onClose, onInput, searchButtonRef]);
}

We import this function in src/theme/SearchBar/index.js and use it here:

https://github.com/ClickHouse/clickhouse-docs/blob/1c5f84e9d4ff6355684f65f377e96922230214a4/src/theme/SearchBar/index.js#L176-L182

Blargian avatar Feb 19 '25 20:02 Blargian

Hmm, great point @Blargian. So this bug should happen from Algolia? What's your thinking about that??

gneo0 avatar Feb 20 '25 16:02 gneo0

Hmm, great point @Blargian. So this bug should happen from Algolia? What's your thinking about that??

Possible but I'm thinking more likely something wrong with the way we are using the component. DocSearch is quite commonly used so I would imagine this issue would be raised already if it was an algolia bug.

Blargian avatar Feb 20 '25 18:02 Blargian