mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: memoize tool toggles and drop dead steering code
The composer palette derives its whole catalog from the badge-row context, and `useToolToggle` returned a fresh object every render, so memoizing that context value changed nothing: six new identities per render kept it invalidating on every keystroke. Memoize the hook's own return instead. Also removes four steer helpers, the run-end map they read and the drain re-arm they called, none of which have had a caller since the pending steers moved into the thread; tells the user why a queued message would not come back to an occupied composer instead of leaving the button looking broken; and refuses a stop once the take is already being transcribed, where it could only rewrite how a committed take gets spent.
This commit is contained in:
parent
d62ab93c7d
commit
e0e9eeee88
9 changed files with 97 additions and 22 deletions
|
|
@ -428,7 +428,12 @@ function Bar({
|
|||
<RoundButton
|
||||
label={dictating ? localize('com_ui_stop') : localize('com_ui_use_micrphone')}
|
||||
onClick={dictating ? dictation.stopToComposer : dictation.start}
|
||||
disabled={speechDisabled && !dictating}
|
||||
/* Once the take is handed to the transcriber there is nothing left
|
||||
to stop, and pressing either of these again would only rewrite
|
||||
how a take that has already been committed gets spent. Cancel,
|
||||
on the `+`, stays live: an external transcription in flight can
|
||||
still be thrown away. */
|
||||
disabled={(speechDisabled && !dictating) || dictation.transcribing}
|
||||
>
|
||||
{dictating ? (
|
||||
<Square className="size-4 fill-current" aria-hidden="true" />
|
||||
|
|
@ -442,6 +447,7 @@ function Bar({
|
|||
primary
|
||||
label={localize('com_nav_send_message')}
|
||||
onClick={dictation.stopAndSend}
|
||||
disabled={dictation.transcribing}
|
||||
>
|
||||
<SendIcon size={18} />
|
||||
</RoundButton>
|
||||
|
|
|
|||
|
|
@ -1,8 +1,8 @@
|
|||
import { memo, useRef, useMemo, useState, useCallback } from 'react';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import { useDrag, useDrop } from 'react-dnd';
|
||||
import { useMediaQuery } from '@librechat/client';
|
||||
import { X, Pencil, GripVertical } from 'lucide-react';
|
||||
import { useMediaQuery, useToastContext } from '@librechat/client';
|
||||
import type { TMessage } from 'librechat-data-provider';
|
||||
import type { SteeringControls, QueuedMessageContext } from '~/hooks/Chat/useSteering';
|
||||
import type { QueuedMessage } from '~/store/families';
|
||||
|
|
@ -71,6 +71,7 @@ function QueueRow({
|
|||
onAnnounce,
|
||||
}: QueueRowProps) {
|
||||
const localize = useLocalize();
|
||||
const { showToast } = useToastContext();
|
||||
const rowRef = useRef<HTMLDivElement>(null);
|
||||
const gripRef = useRef<HTMLButtonElement>(null);
|
||||
const { reorderQueued, restoreQueuedOrder } = steering;
|
||||
|
|
@ -241,7 +242,15 @@ function QueueRow({
|
|||
);
|
||||
if (restored) {
|
||||
steering.removeQueued(message.id);
|
||||
return;
|
||||
}
|
||||
/* Refusing silently reads as a dead button: the row stays, nothing
|
||||
moves, and the reason (a draft in the box, another chat on screen)
|
||||
is somewhere the click was not. */
|
||||
showToast({
|
||||
message: localize('com_ui_queue_remove_blocked'),
|
||||
status: 'warning',
|
||||
});
|
||||
}}
|
||||
className={ICON_BTN}
|
||||
>
|
||||
|
|
|
|||
|
|
@ -18,6 +18,12 @@ jest.mock('~/hooks', () => ({
|
|||
},
|
||||
}));
|
||||
|
||||
const mockShowToast = jest.fn();
|
||||
jest.mock('@librechat/client', () => ({
|
||||
...jest.requireActual('@librechat/client'),
|
||||
useToastContext: () => ({ showToast: mockShowToast }),
|
||||
}));
|
||||
|
||||
const CONVO_ID = 'convo-1';
|
||||
const mockSendQueuedNow = jest.fn();
|
||||
const mockRemoveQueued = jest.fn();
|
||||
|
|
@ -184,6 +190,11 @@ describe('Queue', () => {
|
|||
fireEvent.click(screen.getByLabelText('com_ui_remove_queued'));
|
||||
expect(onRestore).toHaveBeenCalled();
|
||||
expect(mockRemoveQueued).not.toHaveBeenCalled();
|
||||
/* Keeping the words is right; saying nothing about it is not. Without this
|
||||
the row simply does not react and the button reads as broken. */
|
||||
expect(mockShowToast).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ message: 'com_ui_queue_remove_blocked' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('hands the whole message to the composer to edit', () => {
|
||||
|
|
|
|||
|
|
@ -381,12 +381,9 @@ export default function useSteering({
|
|||
visibleConversationRef.current = conversationId;
|
||||
|
||||
/**
|
||||
* How each conversation's last run ended, kept because `useQueueDrain`
|
||||
* CONSUMES the one-shot signal — by the time a reclaim resolves it is already
|
||||
* gone. Every subscriber renders before the drain's effect nulls it, so the
|
||||
* outcome is captured first. Both carriers are watched: the index signal, and
|
||||
* the copy parked under the conversation when the run ended while the user
|
||||
* was looking elsewhere.
|
||||
* The most recent run end seen for a conversation: either the live one for
|
||||
* this pane's index, or the copy parked under the conversation when the run
|
||||
* ended while the user was looking elsewhere.
|
||||
*
|
||||
* Keyed by conversation because this hook is REUSED across chats: a single
|
||||
* slot would answer for whichever chat is on screen when the reclaim lands,
|
||||
|
|
|
|||
|
|
@ -160,6 +160,29 @@ describe('useDictation', () => {
|
|||
expect(ask).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
/* The bar disables stop and send once a take is being transcribed, and the
|
||||
hook refuses them too: a second stop would rewrite how a take that was
|
||||
already committed gets spent. */
|
||||
it('ignores a stop that arrives after the take has ended', async () => {
|
||||
const { result, rerender, currentText } = setup();
|
||||
act(() => result.current.start());
|
||||
mockIsListening = true;
|
||||
act(() => rerender());
|
||||
|
||||
act(() => mockSetTextCallback('already committed'));
|
||||
act(() => result.current.stopToComposer());
|
||||
mockIsListening = false;
|
||||
act(() => rerender());
|
||||
|
||||
mockStop.mockClear();
|
||||
act(() => result.current.stopAndSend());
|
||||
await settle(rerender);
|
||||
|
||||
expect(mockStop).not.toHaveBeenCalled();
|
||||
expect(ask).not.toHaveBeenCalled();
|
||||
expect(currentText()).toBe('already committed');
|
||||
});
|
||||
|
||||
it('keeps the draft when a cancelled external transcription lands anyway', async () => {
|
||||
mockSpeechEndpoint = 'external';
|
||||
const { result, rerender, currentText } = setup({ draft: 'my unsent draft' });
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
import type { TranslationKeys } from '~/hooks/useLocalize';
|
||||
import type { LocalizeFunction } from '~/common';
|
||||
import { isMacPlatform } from '~/utils/shortcuts';
|
||||
import useLocalize from '~/hooks/useLocalize';
|
||||
|
|
|
|||
|
|
@ -201,8 +201,17 @@ export default function useDictation({
|
|||
startRecording();
|
||||
}, [getValues, startRecording]);
|
||||
|
||||
/* Only a running take can be stopped. The bar disables these controls once a
|
||||
transcription is in flight, and this is the same rule stated where the mode
|
||||
is actually written: a second stop would otherwise rewrite how a take that
|
||||
was already committed gets spent. */
|
||||
const activeRef = useRef(active);
|
||||
activeRef.current = active;
|
||||
const stopWith = useCallback(
|
||||
(mode: StopMode) => {
|
||||
if (!activeRef.current) {
|
||||
return;
|
||||
}
|
||||
modeRef.current = mode;
|
||||
setSettling(true);
|
||||
if (mode === 'send') {
|
||||
|
|
|
|||
|
|
@ -162,17 +162,37 @@ export function useToolToggle({
|
|||
[handleChange],
|
||||
);
|
||||
|
||||
return {
|
||||
toggleState: toolValue, // Return the actual value from ephemeralAgent
|
||||
handleChange,
|
||||
isToolEnabled,
|
||||
toolValue,
|
||||
setToggleState: (value: ToolValue) => handleChange({ value }), // Adapter for direct setting
|
||||
ephemeralAgent,
|
||||
debouncedChange,
|
||||
setEphemeralAgent,
|
||||
authData: authQuery?.data,
|
||||
isPinned,
|
||||
setIsPinned,
|
||||
};
|
||||
const setToggleState = useCallback((value: ToolValue) => handleChange({ value }), [handleChange]);
|
||||
|
||||
/* Memoized because `BadgeRowContext` puts six of these in a context value that
|
||||
the composer palette derives its whole catalog from: a fresh object here on
|
||||
every render made that value change on every keystroke, whatever the
|
||||
provider did about it. */
|
||||
return useMemo(
|
||||
() => ({
|
||||
toggleState: toolValue, // Return the actual value from ephemeralAgent
|
||||
handleChange,
|
||||
isToolEnabled,
|
||||
toolValue,
|
||||
setToggleState, // Adapter for direct setting
|
||||
ephemeralAgent,
|
||||
debouncedChange,
|
||||
setEphemeralAgent,
|
||||
authData: authQuery?.data,
|
||||
isPinned,
|
||||
setIsPinned,
|
||||
}),
|
||||
[
|
||||
toolValue,
|
||||
handleChange,
|
||||
isToolEnabled,
|
||||
setToggleState,
|
||||
ephemeralAgent,
|
||||
debouncedChange,
|
||||
setEphemeralAgent,
|
||||
authQuery?.data,
|
||||
isPinned,
|
||||
setIsPinned,
|
||||
],
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1621,6 +1621,7 @@
|
|||
"com_ui_question_unanswered": "No answer was given",
|
||||
"com_ui_queue": "Queue",
|
||||
"com_ui_queue_moved": "Moved to {{0}} of {{1}}",
|
||||
"com_ui_queue_remove_blocked": "Clear the message box in this chat to take this message back.",
|
||||
"com_ui_queue_reorder": "Reorder message, {{0}} of {{1}}",
|
||||
"com_ui_queue_reorder_hint": "Use the up and down arrow keys to move this message in the queue.",
|
||||
"com_ui_queue_send": "Queue message for after the response",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue