From a24653f01cbfc745dcdd3d066192005b9fa85485 Mon Sep 17 00:00:00 2001 From: Divyanshu Gupta Date: Fri, 21 Mar 2025 01:41:11 +0530 Subject: [PATCH 1/3] Fix warnings regarding dependency arrays --- .../demos/EmbeddedComparisonChatbot.tsx | 115 +++++++++--------- .../src/Message/UserFeedback/UserFeedback.tsx | 2 +- .../UserFeedback/UserFeedbackComplete.tsx | 2 +- .../src/MessageBar/MicrophoneButton.tsx | 2 +- packages/module/src/MessageBox/MessageBox.tsx | 10 +- 5 files changed, 65 insertions(+), 66 deletions(-) diff --git a/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx b/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx index b55de504..972f2abd 100644 --- a/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx +++ b/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx @@ -36,71 +36,70 @@ export const CompareChild = ({ name, input, hasNewInput, setIsSendButtonDisabled return id.toString(); }; - const handleSend = (input: string) => { - const date = new Date(); - const newMessages: MessageProps[] = []; - messages.forEach((message) => newMessages.push(message)); - newMessages.push({ - avatar: userAvatar, - avatarProps: { isBordered: true }, - id: generateId(), - name: 'You', - role: 'user', - content: input, - timestamp: `${date?.toLocaleDateString()} ${date?.toLocaleTimeString()}` - }); - newMessages.push({ - avatar: patternflyAvatar, - id: generateId(), - name, - role: 'bot', - timestamp: `${date?.toLocaleDateString()} ${date?.toLocaleTimeString()}`, - isLoading: true - }); - setMessages(newMessages); - // make announcement to assistive devices that new messages have been added - setAnnouncement(`Message from You: ${input}. Message from ${name} is loading.`); - - // this is for demo purposes only; in a real situation, there would be an API response we would wait for - setTimeout(() => { - const loadedMessages: MessageProps[] = []; - // we can't use structuredClone since messages contains functions, but we can't mutate - // items that are going into state or the UI won't update correctly - newMessages.forEach((message) => loadedMessages.push(message)); - loadedMessages.pop(); - loadedMessages.push({ + React.useEffect(() => { + const handleSend = (input: string) => { + const date = new Date(); + const newMessages: MessageProps[] = []; + messages.forEach((message) => newMessages.push(message)); + newMessages.push({ + avatar: userAvatar, + avatarProps: { isBordered: true }, id: generateId(), - role: 'bot', - content: `API response from ${name} goes here`, - name, + name: 'You', + role: 'user', + content: input, + timestamp: `${date?.toLocaleDateString()} ${date?.toLocaleTimeString()}` + }); + newMessages.push({ avatar: patternflyAvatar, - isLoading: false, - actions: { - // eslint-disable-next-line no-console - positive: { onClick: () => console.log('Good response') }, - // eslint-disable-next-line no-console - negative: { onClick: () => console.log('Bad response') }, - // eslint-disable-next-line no-console - copy: { onClick: () => console.log('Copy') }, - // eslint-disable-next-line no-console - share: { onClick: () => console.log('Share') }, - // eslint-disable-next-line no-console - listen: { onClick: () => console.log('Listen') } - }, - timestamp: date.toLocaleString() + id: generateId(), + name, + role: 'bot', + timestamp: `${date?.toLocaleDateString()} ${date?.toLocaleTimeString()}`, + isLoading: true }); - setMessages(loadedMessages); - // make announcement to assistive devices that new message has loaded - setAnnouncement(`Message from ${name}: API response goes here`); - setIsSendButtonDisabled(false); - }, 5000); - }; + setMessages(newMessages); + // make announcement to assistive devices that new messages have been added + setAnnouncement(`Message from You: ${input}. Message from ${name} is loading.`); - React.useEffect(() => { + // this is for demo purposes only; in a real situation, there would be an API response we would wait for + setTimeout(() => { + const loadedMessages: MessageProps[] = []; + // we can't use structuredClone since messages contains functions, but we can't mutate + // items that are going into state or the UI won't update correctly + newMessages.forEach((message) => loadedMessages.push(message)); + loadedMessages.pop(); + loadedMessages.push({ + id: generateId(), + role: 'bot', + content: `API response from ${name} goes here`, + name, + avatar: patternflyAvatar, + isLoading: false, + actions: { + // eslint-disable-next-line no-console + positive: { onClick: () => console.log('Good response') }, + // eslint-disable-next-line no-console + negative: { onClick: () => console.log('Bad response') }, + // eslint-disable-next-line no-console + copy: { onClick: () => console.log('Copy') }, + // eslint-disable-next-line no-console + share: { onClick: () => console.log('Share') }, + // eslint-disable-next-line no-console + listen: { onClick: () => console.log('Listen') } + }, + timestamp: date.toLocaleString() + }); + setMessages(loadedMessages); + // make announcement to assistive devices that new message has loaded + setAnnouncement(`Message from ${name}: API response goes here`); + setIsSendButtonDisabled(false); + }, 5000); + }; if (input) { handleSend(input); } - }, [hasNewInput]); + }, [hasNewInput, input, messages, name, setIsSendButtonDisabled]); // Auto-scrolls to the latest message React.useEffect(() => { diff --git a/packages/module/src/Message/UserFeedback/UserFeedback.tsx b/packages/module/src/Message/UserFeedback/UserFeedback.tsx index 2bb5bd89..d77a73f9 100644 --- a/packages/module/src/Message/UserFeedback/UserFeedback.tsx +++ b/packages/module/src/Message/UserFeedback/UserFeedback.tsx @@ -81,7 +81,7 @@ const UserFeedback: React.FunctionComponent = ({ if (focusOnLoad) { divRef.current?.focus(); } - }, []); + }); return ( /* card does not have ref forwarding; hence wrapper div */ diff --git a/packages/module/src/Message/UserFeedback/UserFeedbackComplete.tsx b/packages/module/src/Message/UserFeedback/UserFeedbackComplete.tsx index b545f3b0..74274833 100644 --- a/packages/module/src/Message/UserFeedback/UserFeedbackComplete.tsx +++ b/packages/module/src/Message/UserFeedback/UserFeedbackComplete.tsx @@ -77,7 +77,7 @@ const UserFeedbackComplete: React.FunctionComponent = if (focusOnLoad) { divRef.current?.focus(); } - }, []); + }); React.useEffect(() => { const calculatedTimeout = timeout === true ? 8000 : Number(timeout); diff --git a/packages/module/src/MessageBar/MicrophoneButton.tsx b/packages/module/src/MessageBar/MicrophoneButton.tsx index d71c8451..8b37fedd 100644 --- a/packages/module/src/MessageBar/MicrophoneButton.tsx +++ b/packages/module/src/MessageBar/MicrophoneButton.tsx @@ -81,7 +81,7 @@ export const MicrophoneButton: React.FunctionComponent = setSpeechRecognition(recognition); } - }, [onSpeechRecognition]); + }, [onSpeechRecognition, language, onIsListeningChange]); if (!speechRecognition) { return null; diff --git a/packages/module/src/MessageBox/MessageBox.tsx b/packages/module/src/MessageBox/MessageBox.tsx index 3604664e..4051582f 100644 --- a/packages/module/src/MessageBox/MessageBox.tsx +++ b/packages/module/src/MessageBox/MessageBox.tsx @@ -46,7 +46,7 @@ const MessageBoxBase: React.FunctionComponent = ({ setAtTop(scrollTop === 0); setAtBottom(Math.round(scrollTop) + Math.round(clientHeight) >= Math.round(scrollHeight) - 1); // rounding means it could be within a pixel of the bottom } - }, []); + }, [messageBoxRef]); const checkOverflow = React.useCallback(() => { const element = messageBoxRef.current; @@ -54,21 +54,21 @@ const MessageBoxBase: React.FunctionComponent = ({ const { scrollHeight, clientHeight } = element; setIsOverflowing(scrollHeight >= clientHeight); } - }, []); + }, [messageBoxRef]); const scrollToTop = React.useCallback(() => { const element = messageBoxRef.current; if (element) { element.scrollTo({ top: 0, behavior: 'smooth' }); } - }, []); + }, [messageBoxRef]); const scrollToBottom = React.useCallback(() => { const element = messageBoxRef.current; if (element) { element.scrollTo({ top: element.scrollHeight, behavior: 'smooth' }); } - }, []); + }, [messageBoxRef]); // Detect scroll position React.useEffect(() => { @@ -85,7 +85,7 @@ const MessageBoxBase: React.FunctionComponent = ({ element.removeEventListener('scroll', handleScroll); }; } - }, [checkOverflow, handleScroll]); + }, [checkOverflow, handleScroll, messageBoxRef]); return ( <> From 2adb5623affdf15b5fea635fff1bcbf07ad75ce5 Mon Sep 17 00:00:00 2001 From: Divyanshu Gupta Date: Fri, 21 Mar 2025 01:50:44 +0530 Subject: [PATCH 2/3] wrapped handle send function inside useCallback --- .../examples/demos/EmbeddedComparisonChatbot.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx b/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx index 972f2abd..28bb5d66 100644 --- a/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx +++ b/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx @@ -36,8 +36,8 @@ export const CompareChild = ({ name, input, hasNewInput, setIsSendButtonDisabled return id.toString(); }; - React.useEffect(() => { - const handleSend = (input: string) => { + const handleSend = React.useCallback( + (input: string) => { const date = new Date(); const newMessages: MessageProps[] = []; messages.forEach((message) => newMessages.push(message)); @@ -95,11 +95,15 @@ export const CompareChild = ({ name, input, hasNewInput, setIsSendButtonDisabled setAnnouncement(`Message from ${name}: API response goes here`); setIsSendButtonDisabled(false); }, 5000); - }; + }, + [messages, name, setIsSendButtonDisabled] + ); + + React.useEffect(() => { if (input) { handleSend(input); } - }, [hasNewInput, input, messages, name, setIsSendButtonDisabled]); + }, [hasNewInput, input, handleSend]); // Auto-scrolls to the latest message React.useEffect(() => { From 5444690496d21f556b3a38ba70859dae0c501e3f Mon Sep 17 00:00:00 2001 From: Divyanshu Gupta Date: Wed, 9 Apr 2025 23:40:16 +0530 Subject: [PATCH 3/3] removed breaking changes --- .../chatbot/examples/demos/EmbeddedComparisonChatbot.tsx | 2 +- packages/module/src/Message/UserFeedback/UserFeedback.tsx | 2 +- .../module/src/Message/UserFeedback/UserFeedbackComplete.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx b/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx index 28bb5d66..71676833 100644 --- a/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx +++ b/packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx @@ -103,7 +103,7 @@ export const CompareChild = ({ name, input, hasNewInput, setIsSendButtonDisabled if (input) { handleSend(input); } - }, [hasNewInput, input, handleSend]); + }, [hasNewInput, input]); // Auto-scrolls to the latest message React.useEffect(() => { diff --git a/packages/module/src/Message/UserFeedback/UserFeedback.tsx b/packages/module/src/Message/UserFeedback/UserFeedback.tsx index d77a73f9..2bb5bd89 100644 --- a/packages/module/src/Message/UserFeedback/UserFeedback.tsx +++ b/packages/module/src/Message/UserFeedback/UserFeedback.tsx @@ -81,7 +81,7 @@ const UserFeedback: React.FunctionComponent = ({ if (focusOnLoad) { divRef.current?.focus(); } - }); + }, []); return ( /* card does not have ref forwarding; hence wrapper div */ diff --git a/packages/module/src/Message/UserFeedback/UserFeedbackComplete.tsx b/packages/module/src/Message/UserFeedback/UserFeedbackComplete.tsx index 74274833..b545f3b0 100644 --- a/packages/module/src/Message/UserFeedback/UserFeedbackComplete.tsx +++ b/packages/module/src/Message/UserFeedback/UserFeedbackComplete.tsx @@ -77,7 +77,7 @@ const UserFeedbackComplete: React.FunctionComponent = if (focusOnLoad) { divRef.current?.focus(); } - }); + }, []); React.useEffect(() => { const calculatedTimeout = timeout === true ? 8000 : Number(timeout);