(DEVHUB-1898, UP-6913) Add Hotkey Functionality #506
Conversation
| @@ -40,6 +40,7 @@ export function DevCenterChatbot(props: DevCenterChatbotProps) { | |||
| /> | |||
| </> | |||
| ), | |||
| shouldUseHotkey: true, | |||
There was a problem hiding this comment.
I know we aren't using this component, but I set this to true here so it would show up in the tester
docs/docs/ui.md
Outdated
| @@ -124,4 +124,5 @@ The `<ModalView />` component renders a chat message feed in a modal window. It | |||
| | `initialMessageText` | `string?` | The text content of an initial assistant message at the top of the message feed. | If no text is specified, then no message is shown. | | |||
| | `inputBarPlaceholder` | `string?` | The placeholder text shown when the input bar is empty. | If not specified, the input bar uses default placeholders. | | |||
| | `inputBottomText` | `string?` | Text that appears immediately below the input bar. | If not specified, no bottom text is shown. | | |||
| | `shouldUseHotkey` | `boolean?` | If `true`, the chat window will show on hotkey press (`/`) | `false` | | |||
There was a problem hiding this comment.
perhaps rename to 'shouldUseSlashHotkey` to be more specific.
alternatively, could make sense to let the user define what the hotkey is. then user could set it to "/", "s", etc
There was a problem hiding this comment.
+1 for customizable hotkey. I think ideally hotkey stuff would be handled outside of our library, but we can add it as a convenience for this case.
Taking a step back, I think the most composable pattern is to present this as a new type of trigger component, a HotkeyTrigger. You can use this alongside the input bar trigger inside of the chatbot provider.
function MyApp() {
return (
<Chatbot>
<HotkeyTrigger onKey="/" />
<DevCenterInputBarTrigger />
</Chatbot>
)
}Let me know if you have thoughts / questions on how to implement!
There was a problem hiding this comment.
I can go ahead and rename it to shouldUseSlashHotkey. I would lean towards this option because the slash in the indicator is hardcoded in leafygreen (with the justification being to keep the usage consistent across different sites' implementations)
edit: @nlarew Sorry didn't see your comment. If we went this route I think there would need to be follow up work in leafygreen, as currently the visual indicator (and focus listener) are hardcoded to /.
There was a problem hiding this comment.
Let's quietly ship shouldUseSlashHotkey as a Chatbot prop and then remove it in favor of a more customizable trigger.
We have a bit of a tight deadline to get the chatbot on DevCenter/University--UAT starts next Tuesday, and I would still need to raise a couple of PRs to our repos following this one being merged. Is there a possibility that we can get this merged, and then if Nick has any suggestions when he returns I can create a follow-up PR, or something along those lines? |
I am commenting on Max's comment on whether we can try to merge this. It would be great if someone could approve it in Nick's absence so that it can get merged. If we're concerned about cutting a release that may break someone else's experience, we could cut a release just for our project on university and developer center. |
given the tight deadline, let me make a pre-release that you can pull from npm. Will just make from this branch so nothing touches |
Jira: https://jira.mongodb.org/browse/DEVHUB-1898, https://jira.mongodb.org/browse/UP-6913
Changes
shouldUseHotkeyprop on the ModalView and ChatWindow components that controls whether pressing the hotkey (/) will open the window, and whether the hotkey indicator is shown on the input bar.Notes
shouldUseHotkeybut open to suggestions if we want to change this