-
Notifications
You must be signed in to change notification settings - Fork 141
fix: support cmd+k to open the search modal
#1755
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
Conversation
also opens the kapa widget with the AI chat tab first
|
Preview for this PR was built for commit |
TC-MO
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tested both windows & mac
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Search Modal Stuck Open on Widget Failures
The opened state, which prevents re-opening the search modal, is incorrectly left as true if the modal fails to open. This occurs when the Kapa or Inkeep widgets are unavailable, or an unknown search variant is encountered, as the onClick handler exits without resetting the state. Consequently, the search modal cannot be opened again until a page refresh.
apify-docs-theme/src/theme/SearchBar/index.js#L200-L203
apify-docs/apify-docs-theme/src/theme/SearchBar/index.js
Lines 200 to 203 in b48a83b
| modal.update({ modalSettings: { isOpen: true } }); | |
| } else { | |
| console.error('Inkeep widget is not available.'); | |
| } |
apify-docs-theme/src/theme/SearchBar/index.js#L119-L123
apify-docs/apify-docs-theme/src/theme/SearchBar/index.js
Lines 119 to 123 in b48a83b
| if (variant !== 'inkeep') { | |
| console.warn('Unknown search variant:', variant); | |
| return; | |
| } |
Comment bugbot run to trigger another review on this PR
|
|
||
| function handleOpenChange(newOpen) { | ||
| modal.update({ modalSettings: { isOpen: newOpen } }); | ||
| setOpened(newOpen); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: SearchBar Prop Ignored, State Not Reset
The SearchBar component reassigns its onClick prop, effectively ignoring any onClick handler passed from parent components. Additionally, the opened state, which prevents multiple modal openings, is not reset to false if the Kapa or Inkeep search widgets fail to open. This leaves the search bar permanently unresponsive until the page is refreshed.
| window.Kapa.open(); | ||
| window.Kapa('onModalClose', () => { | ||
| setOpened(false); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Kapa.ai Integration Causes Search Bar Issues
The search bar component exhibits two issues related to the Kapa.ai integration:
- Opening the Kapa modal repeatedly registers new
onModalCloseevent listeners, leading to multiplesetOpened(false)calls and potential memory leaks when the modal closes. - If the Kapa.ai widget is unavailable, the
openedstate is set totruebut never reset tofalse, permanently preventing any subsequent attempts to open the search modal.
Also opens the kapa widget with the AI chat tab first. Closes apify#1745
Also opens the kapa widget with the AI chat tab first.
Closes #1745