Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions workspaces/lightspeed/.changeset/few-jars-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@red-hat-developer-hub/backstage-plugin-lightspeed': patch
---

Added Escape key support to cycle through display modes. Pressing Escape now transitions through Fullscreen → Docked → Overlay → Close. When the Display Mode settings dropdown is open, the first Escape closes the dropdown, and subsequent Escape presses cycle the display mode.
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ export const LightspeedChat = ({
setCurrentConversationId,
draftMessage,
setDraftMessage,
setIsSettingsDropdownOpen,
} = useLightspeedDrawerContext();
const isFullscreenMode = displayMode === ChatbotDisplayMode.embedded;
const [isChatHistoryDrawerOpen, setIsChatHistoryDrawerOpen] =
Expand Down Expand Up @@ -737,6 +738,7 @@ export const LightspeedChat = ({
setDisplayMode={setDisplayMode}
displayMode={displayMode}
onPinnedChatsToggle={handlePinningChatsToggle}
setIsSettingsDropdownOpen={setIsSettingsDropdownOpen}
/>
</ChatbotHeader>
<Divider />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type LightspeedChatBoxHeaderProps = {
onPinnedChatsToggle: (state: boolean) => void;
isModelSelectorDisabled?: boolean;
setDisplayMode: (mode: ChatbotDisplayMode) => void;
setIsSettingsDropdownOpen: (isOpen: boolean) => void;
};

const useStyles = makeStyles(theme =>
Expand Down Expand Up @@ -83,8 +84,10 @@ export const LightspeedChatBoxHeader = ({
onPinnedChatsToggle,
isModelSelectorDisabled = false,
setDisplayMode,
setIsSettingsDropdownOpen,
}: LightspeedChatBoxHeaderProps) => {
const [isOptionsMenuOpen, setIsOptionsMenuOpen] = useState(false);
const [isSettingsMenuOpen, setIsSettingsMenuOpen] = useState(false);
const { t } = useTranslation();

const styles = useStyles();
Expand Down Expand Up @@ -136,6 +139,19 @@ export const LightspeedChatBoxHeader = ({
setDisplayMode(ChatbotDisplayMode.default);
};

// Toggle settings menu (called when clicking the toggle button)
const handleSettingsMenuToggle = () => {
const newState = !isSettingsMenuOpen;
setIsSettingsMenuOpen(newState);
setIsSettingsDropdownOpen(newState);
};

// Handle settings menu close (called on Escape or click outside)
const handleSettingsMenuClose = (isOpen: boolean) => {
setIsSettingsMenuOpen(isOpen);
setIsSettingsDropdownOpen(isOpen);
};

return (
<ChatbotHeaderActions>
<Dropdown
Expand Down Expand Up @@ -177,9 +193,12 @@ export const LightspeedChatBoxHeader = ({
</Dropdown>
<ChatbotHeaderOptionsDropdown
isCompact
isOpen={isSettingsMenuOpen}
onOpenChange={handleSettingsMenuClose}
toggleProps={{
'aria-label': t('aria.settings.label'),
className: styles.optionsToggle,
onClick: handleSettingsMenuToggle,
}}
tooltipProps={{
content: t('tooltip.settings'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ export interface LightspeedDrawerContextType {
* To save file attachments as a draft when switching modes
*/
setDraftFileContents: (files: FileContent[]) => void;
/**
* Whether the settings dropdown is currently open
* Used to prevent Escape key from cycling display modes when dropdown is open
*/
isSettingsDropdownOpen: boolean;
/**
* Setter for settings dropdown open state
*/
setIsSettingsDropdownOpen: (isOpen: boolean) => void;
Comment on lines +84 to +88
Copy link
Member

Choose a reason for hiding this comment

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

This is related to my previous comment.

Can you use similar approach to track if the rename/delete/attachment modals are open so that you can use that information to see if any modals are open, rather than relying on className .MuiDialog-root ?

pseudo code:

  const [openModals, setOpenModals] = useState<Set<string>>(new Set());

 const openModal = useCallback((modalId: string) => {
    setOpenModals(prev => new Set(prev).add(modalId));
  }, []);

  const closeModal = useCallback((modalId: string) => {
    setOpenModals(prev => {
      const next = new Set(prev);
      next.delete(modalId);
      return next;
    });
    

Then context can expose method and properties like openModal, closeModal, hasOpenModals etc, so that you can use this to decide whether to perform any action or not.

Copy link
Member

Choose a reason for hiding this comment

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

The above code can become complex, see my latest comment below, we dont need to maintain modalIds instead we can have a simple boolean to track if there any nestedModals Open and based on that we can avoid closing the main chatbot.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
const [draftFileContents, setDraftFileContentsState] = useState<
FileContent[]
>([]);
const [isSettingsDropdownOpen, setIsSettingsDropdownOpen] =
useState<boolean>(false);
const openedViaFABRef = useRef<boolean>(false);

const isLightspeedRoute = location.pathname.startsWith('/lightspeed');
Expand Down Expand Up @@ -188,6 +190,57 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
],
);

// Cycle display mode on Escape: Fullscreen → Docked → Overlay → Close
const cycleDisplayModeOnEscape = useCallback(() => {
switch (displayModeState) {
case ChatbotDisplayMode.embedded: // Fullscreen → Docked
setDisplayMode(ChatbotDisplayMode.docked);
break;
case ChatbotDisplayMode.docked: // Docked → Overlay
setDisplayMode(ChatbotDisplayMode.default);
break;
case ChatbotDisplayMode.default: // Overlay → Close
closeChatbot();
break;
default:
break;
}
}, [displayModeState, setDisplayMode, closeChatbot]);

// Handle ChatbotModal close (overlay mode only)
// Only cycle display mode if no dropdown is open
const handleModalClose = useCallback(() => {
if (isSettingsDropdownOpen) {
// Settings dropdown is open, let it close first
// Don't cycle display mode on this Escape press
return;
}
cycleDisplayModeOnEscape();
}, [isSettingsDropdownOpen, cycleDisplayModeOnEscape]);

// Global Escape key listener for display mode cycling
useEffect(() => {
const handleKeyDown = (event: KeyboardEvent) => {
if (event.key !== 'Escape' || !isOpen) {
return;
}

// Check if any modal dialog is currently visible (Delete, Rename, etc.)
const hasOpenModal = document.querySelector('.MuiDialog-root');

// If settings dropdown or modal is open, let those handle Escape first
if (isSettingsDropdownOpen || hasOpenModal) {
return;
}
Comment on lines +228 to +234
Copy link
Member

Choose a reason for hiding this comment

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

This is a brittle implementation and could break if MUI class names are changed or if we start using BUI instead of MUI.

const hasOpenModal = document.querySelector('.MuiDialog-root');


event.preventDefault();
cycleDisplayModeOnEscape();
};

document.addEventListener('keydown', handleKeyDown);
return () => document.removeEventListener('keydown', handleKeyDown);
}, [isOpen, isSettingsDropdownOpen, cycleDisplayModeOnEscape]);

// Only render ChatbotModal for overlay mode
// Docked mode is handled by ApplicationDrawer in Root
// Embedded mode is handled by LightspeedPage route
Expand All @@ -210,6 +263,8 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
setDraftMessage,
draftFileContents,
setDraftFileContents,
isSettingsDropdownOpen,
setIsSettingsDropdownOpen,
}),
[
isOpen,
Expand All @@ -224,6 +279,8 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
setDraftMessage,
draftFileContents,
setDraftFileContents,
isSettingsDropdownOpen,
setIsSettingsDropdownOpen,
],
);

Expand All @@ -234,7 +291,7 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
<ChatbotModal
isOpen
displayMode={displayModeState}
onClose={closeChatbot}
onClose={handleModalClose}
ouiaId="LightspeedChatbotModal"
aria-labelledby="lightspeed-chatpopup-modal"
className={classes.chatbotModal}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ describe('LightspeedChat', () => {
setDraftMessage: jest.fn(),
draftFileContents: [],
setDraftFileContents: jest.fn(),
isSettingsDropdownOpen: false,
setIsSettingsDropdownOpen: jest.fn(),
});

localStorage.clear();
Expand Down Expand Up @@ -508,6 +510,8 @@ describe('LightspeedChat', () => {
setDraftMessage: jest.fn(),
draftFileContents: [],
setDraftFileContents: jest.fn(),
isSettingsDropdownOpen: false,
setIsSettingsDropdownOpen: jest.fn(),
});

render(setupLightspeedChat());
Expand Down Expand Up @@ -539,6 +543,8 @@ describe('LightspeedChat', () => {
setDraftMessage: jest.fn(),
draftFileContents: [],
setDraftFileContents: jest.fn(),
isSettingsDropdownOpen: false,
setIsSettingsDropdownOpen: jest.fn(),
});

render(setupLightspeedChat());
Expand Down Expand Up @@ -570,6 +576,8 @@ describe('LightspeedChat', () => {
setDraftMessage: jest.fn(),
draftFileContents: [],
setDraftFileContents: jest.fn(),
isSettingsDropdownOpen: false,
setIsSettingsDropdownOpen: jest.fn(),
});

render(setupLightspeedChat());
Expand Down
Loading