Skip to content

feat(Tour): feat: support keyboard operation #87

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

feat(Tour): feat: support keyboard operation #87

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e891e70
feat(Tour): feat: support keyboard operation
Linkodt Oct 2, 2025
5658930
feat(Tour): feat: support keyboard operation
Linkodt Oct 2, 2025
2a6a8fd
feat(Tour): feat: support keyboard operation
Linkodt Oct 4, 2025
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
8 changes: 8 additions & 0 deletions docs/demo/keyboard.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
title: keyboard
nav:
title: Demo
path: /demo
---

<code src="../examples/keyboard.tsx"></code>
52 changes: 52 additions & 0 deletions docs/examples/keyboard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import React, { useRef } from 'react';
import Tour from '../../src/index';
import './basic.less';

const App = () => {
const firstBtnRef = useRef<HTMLButtonElement>(null);
const secondBtnRef = useRef<HTMLButtonElement>(null);
const thirdBtnRef = useRef<HTMLButtonElement>(null);
return (
<React.StrictMode>
<div style={{ margin: 20 }}>
<div>
<button
className="ant-target"
ref={firstBtnRef}
>
One
</button>
<button className="ant-target" ref={secondBtnRef}>
Two
</button>
<button className="ant-target" ref={thirdBtnRef}>
Three
</button>
</div>

<div style={{ height: 200 }} />

<Tour
defaultCurrent={0}
keyboard={true}
steps={[
{
title: 'One',
target: () => firstBtnRef.current,
},
{
title: 'Two',
target: () => secondBtnRef.current,
},
{
title: 'Three',
target: () => thirdBtnRef.current,
},
]}
/>
Comment on lines +29 to +46
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

示例缺少 Tour 可见性控制。

当前示例中 Tour 组件始终处于打开状态（依赖 defaultCurrent 但没有 open prop 控制）。这可能让用户难以理解如何在实际应用中初始打开和关闭 Tour。

建议添加状态控制和触发按钮来演示完整的使用场景：

+import React, { useRef, useState } from 'react';
-import React, { useRef } from 'react';
 import Tour from '../../src/index';
 import './basic.less';

 const App = () => {
+  const [open, setOpen] = useState(false);
   const firstBtnRef = useRef<HTMLButtonElement>(null);
   const secondBtnRef = useRef<HTMLButtonElement>(null);
   const thirdBtnRef = useRef<HTMLButtonElement>(null);
   return (
     <React.StrictMode>
     <div style={{ margin: 20 }}>
+      <button onClick={() => setOpen(true)}>开始导览</button>
       <div>
         <button
           className="ant-target"
           ref={firstBtnRef}
         >
           One
         </button>
         {/* ... */}
       </div>

       <Tour
+        open={open}
+        onClose={() => setOpen(false)}
         defaultCurrent={0}
         keyboard={true}
         steps={[
           {/* ... */}
         ]}
       />
     </div>
     </React.StrictMode>
   );
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Tour
defaultCurrent={0}
keyboard={true}
steps={[
{
title: 'One',
target: () => firstBtnRef.current,
},
{
title: 'Two',
target: () => secondBtnRef.current,
},
{
title: 'Three',
target: () => thirdBtnRef.current,
},
]}
/>
import React, { useRef, useState } from 'react';
import Tour from '../../src/index';
import './basic.less';
const App = () => {
const [open, setOpen] = useState(false);
const firstBtnRef = useRef<HTMLButtonElement>(null);
const secondBtnRef = useRef<HTMLButtonElement>(null);
const thirdBtnRef = useRef<HTMLButtonElement>(null);
return (
<React.StrictMode>
<div style={{ margin: 20 }}>
<button onClick={() => setOpen(true)}>开始导览</button>
<div>
<button
className="ant-target"
ref={firstBtnRef}
>
One
</button>
{/* ...other buttons... */}
</div>
<Tour
open={open}
onClose={() => setOpen(false)}
defaultCurrent={0}
keyboard={true}
steps={[
{
title: 'One',
target: () => firstBtnRef.current,
},
{
title: 'Two',
target: () => secondBtnRef.current,
},
{
title: 'Three',
target: () => thirdBtnRef.current,
},
]}
/>
</div>
</React.StrictMode>
);
};
export default App;
🤖 Prompt for AI Agents 
In docs/examples/keyboard.tsx around lines 29 to 46, the Tour is always shown
because it only uses defaultCurrent and lacks open state control; add a boolean
state (e.g., const [open, setOpen] = useState(false)) and pass open={open} and
onClose={() => setOpen(false)} to the Tour, add a trigger button that calls
setOpen(true) to open the tour, and ensure any initial step control still uses
defaultCurrent or useCurrent as needed so the example demonstrates opening,
closing, and triggering the Tour in a real app.

</div>
</React.StrictMode>
);
};

export default App;
52 changes: 47 additions & 5 deletions src/Tour.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const Tour: React.FC<TourProps> = props => {
className,
style,
getPopupContainer,
keyboard = false,
...restProps
} = props;

Expand Down Expand Up @@ -157,17 +158,58 @@ const Tour: React.FC<TourProps> = props => {
return getPlacements(arrowPointAtCenter);
}, [builtinPlacements, arrowPointAtCenter]);

// ================= close ========================
const handleClose = () => {
setMergedOpen(false);
onClose?.(mergedCurrent);
};

// ================= Keyboard Navigation ==============
const handleKeyDown = React.useCallback((e: KeyboardEvent) => {
if (!mergedOpen) {
return;
}
if (e.key === 'ArrowLeft') {
if (mergedCurrent <= 0) {
return;
}
e.preventDefault();
e.stopPropagation();
onInternalChange(mergedCurrent - 1);
return;
}
if (e.key === 'ArrowRight') {
if (mergedCurrent >= steps.length - 1) {
return;
}
e.preventDefault();
e.stopPropagation();
onInternalChange(mergedCurrent + 1);
return;
}
if (e.key === 'Escape') {
e.preventDefault();
e.stopPropagation();
handleClose();
return;
}
}, [mergedCurrent, mergedOpen, steps.length]);
Copy link

Choose a reason for hiding this comment

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

high

The implementation of handleKeyDown has a potential bug and can be improved for readability.

  1. Stale Closures: The useCallback hook for handleKeyDown is missing onInternalChange and handleClose in its dependency array. This can lead to bugs where stale versions of these functions are called if the onChange or onClose props change. Adding them to the dependency array is necessary for correctness.

  2. Readability: The chain of if statements for key handling can be refactored into a switch statement. This improves readability and maintainability.

Important Note on Performance: Adding the missing dependencies will fix the bug, but since onInternalChange and handleClose are not memoized, handleKeyDown will be recreated on every render. This will cause the useEffect at line 198 to remove and re-add the global event listener on every render, which can be a performance concern. The ideal solution would be to also wrap onInternalChange and handleClose in useCallback where they are defined.

  const handleKeyDown = React.useCallback((e: KeyboardEvent) => {
    if (!mergedOpen) {
      return;
    }

    switch (e.key) {
      case 'ArrowLeft':
        if (mergedCurrent > 0) {
          e.preventDefault();
          e.stopPropagation();
          onInternalChange(mergedCurrent - 1);
        }
        break;
      case 'ArrowRight':
        if (mergedCurrent < steps.length - 1) {
          e.preventDefault();
          e.stopPropagation();
          onInternalChange(mergedCurrent + 1);
        }
        break;
      case 'Escape':
        e.preventDefault();
        e.stopPropagation();
        handleClose();
        break;
      default:
        // Do nothing for other keys
        break;
    }
  }, [mergedCurrent, mergedOpen, steps.length, onInternalChange, handleClose]);


React.useEffect(() => {
if (keyboard) {
document.addEventListener('keydown', handleKeyDown);
}
return () => {
document.removeEventListener('keydown', handleKeyDown);
};
}, [handleKeyDown, keyboard]);
Comment on lines 202 to 213
Copy link

Choose a reason for hiding this comment

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

medium

Attaching the event listener directly to document can lead to issues if multiple Tour components are rendered and open at the same time, as they would all react to the keyboard events. While it's unlikely to have multiple tours open simultaneously, a more robust implementation might involve a mechanism to ensure only the top-most or most recently interacted-with tour handles the keyboard events. This is a common pattern in component libraries for managing global listeners for modals and dialogs. This is something to consider for future improvements to prevent potential conflicts in more complex applications.

Comment on lines 202 to 213
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

多个 Tour 实例可能导致键盘事件冲突。

如果页面上同时挂载多个启用 keyboard 的 Tour 实例，所有实例的监听器都会触发，可能导致意外的行为或冲突。建议添加逻辑确保只有当前活动的 Tour 实例处理键盘事件。

可能的解决方案：

  1. 使用事件目标检查或 z-index 比较确定哪个 Tour 应该处理事件
  2. 使用全局状态管理确保只有一个 Tour 监听键盘事件
  3. 在文档中明确说明不支持多个同时打开的 Tour 使用键盘导航

示例修复（检查 Tour 是否打开）：

   const handleKeyDown = React.useCallback((e: KeyboardEvent) => {
-    if (!mergedOpen) {
+    if (!mergedOpen || !keyboard) {
       return;
     }

并在文档中说明此限制。

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/Tour.tsx around lines 198-205, multiple mounted Tour instances that
enable keyboard can all add the same keydown listener causing conflicts; modify
the effect to only attach the listener when this Tour is the active/open one
(e.g., check isOpen/active state or an activeTourId from context/global store)
and include that same condition in the cleanup so only the active instance
handles key events; alternatively register a single global keyboard handler that
routes events to the active Tour by id; update docs to state that keyboard
navigation only applies to the active/open Tour instance.


// ========================= Render =========================
// Skip if not init yet
if (targetElement === undefined || !hasOpened) {
return null;
}

const handleClose = () => {
setMergedOpen(false);
onClose?.(mergedCurrent);
};

const getPopupElement = () => (
<TourStep
styles={styles}
Expand Down
1 change: 1 addition & 0 deletions src/interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,5 @@ export interface TourProps extends Pick<TriggerProps, 'onPopupAlign'> {
arrowPointAtCenter?: boolean;
}) => TriggerProps['builtinPlacements']);
disabledInteraction?: boolean;
keyboard?: boolean;
}