Merge pull request #812 from wise-king-sullyman/add-icon-swapping-on-click

feat(messages): fill positive, negative, and copy action icons on click

Author: thatblindgeye

packages/module/patternfly-docs/content/extensions/chatbot/examples/Messages/MessageWithIconSwapping.tsx

@@ -0,0 +1,22 @@
+import { FunctionComponent } from 'react';
+
+import Message from '@patternfly/chatbot/dist/dynamic/Message';
+import patternflyAvatar from './patternfly_avatar.jpg';
+
+export const IconSwappingExample: FunctionComponent = () => (
+  <Message
+    name="Bot"
+    role="bot"
+    avatar={patternflyAvatar}
+    content="Click the response actions to see the outlined icons swapped with the filled variants!"
+    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('Copied') }
+    }}
+    useFilledIconsOnClick
+  />
+);

packages/module/patternfly-docs/content/extensions/chatbot/examples/Messages/Messages.md

@@ -138,6 +138,17 @@ When `persistActionSelection` is `true`:
 
 ```
 
+### Message actions that fill
+
+To provide enhanced visual feedback when users interact with response actions, you can enable icon swapping by setting `useFilledIconsOnClick` to `true`. When enabled, the predefined "positive" and "negative" actions will automatically swap to their filled icon counterparts when clicked, replacing the original outlined icon variants. 
+
+This is especially useful for actions that are intended to persist (such as the "positive" and "negative" responses), so that a user's selection is more clear and emphasized.
+
+
+```js file="./MessageWithIconSwapping.tsx"
+
+```
+
 ### Multiple messsage action groups
 
 To maintain finer control over message action selection behavior, you can create groups of actions by passing an array of objects to the `actions` prop. This allows you to separate actions into conceptually or functionally different groups and implement different behavior for each group as needed. For example, you could separate feedback actions (thumbs up/down) form utility actions (copy and download), and have different selection behaviors for each group.

packages/module/src/Message/Message.test.tsx

@@ -9,6 +9,23 @@ import rehypeExternalLinks from '../__mocks__/rehype-external-links';
 import { AlertActionLink, Button, CodeBlockAction } from '@patternfly/react-core';
 import { DeepThinkingProps } from '../DeepThinking';
 
+// Mock the icon components
+jest.mock('@patternfly/react-icons', () => ({
+  OutlinedThumbsUpIcon: () => <div>OutlinedThumbsUpIcon</div>,
+  ThumbsUpIcon: () => <div>ThumbsUpIcon</div>,
+  OutlinedThumbsDownIcon: () => <div>OutlinedThumbsDownIcon</div>,
+  ThumbsDownIcon: () => <div>ThumbsDownIcon</div>,
+  OutlinedCopyIcon: () => <div>OutlinedCopyIcon</div>,
+  DownloadIcon: () => <div>DownloadIcon</div>,
+  ExternalLinkAltIcon: () => <div>ExternalLinkAltIcon</div>,
+  VolumeUpIcon: () => <div>VolumeUpIcon</div>,
+  PencilAltIcon: () => <div>PencilAltIcon</div>,
+  CheckIcon: () => <div>CheckIcon</div>,
+  CloseIcon: () => <div>CloseIcon</div>,
+  ExternalLinkSquareAltIcon: () => <div>ExternalLinkSquareAltIcon</div>,
+  TimesIcon: () => <div>TimesIcon</div>
+}));
+
 const ALL_ACTIONS = [
   { label: /Good response/i },
   { label: /Bad response/i },
@@ -1351,4 +1368,51 @@ describe('Message', () => {
     render(<Message alignment="end" avatar="./img" role="user" name="User" content="" />);
     expect(screen.getByRole('region')).toHaveClass('pf-m-end');
   });
+
+  // We're just testing the positive action here to ensure logic passes through as needed, the other actions are
+  // tested in ResponseActions.test.tsx along with other aspects of this functionality
+  it('should not swap icons when useFilledIconsOnClick is omitted', async () => {
+    const user = userEvent.setup();
+
+    render(
+      <Message
+        avatar="./img"
+        role="bot"
+        name="Bot"
+        content="Hi"
+        actions={{
+          positive: { onClick: jest.fn() }
+        }}
+      />
+    );
+
+    expect(screen.getByText('OutlinedThumbsUpIcon')).toBeInTheDocument();
+
+    await user.click(screen.getByRole('button', { name: /Good response/i }));
+
+    expect(screen.getByText('OutlinedThumbsUpIcon')).toBeInTheDocument();
+    expect(screen.queryByText('ThumbsUpIcon')).not.toBeInTheDocument();
+  });
+
+  it('should swap icons when useFilledIconsOnClick is true', async () => {
+    const user = userEvent.setup();
+
+    render(
+      <Message
+        avatar="./img"
+        role="bot"
+        name="Bot"
+        content="Hi"
+        actions={{
+          positive: { onClick: jest.fn() }
+        }}
+        useFilledIconsOnClick
+      />
+    );
+
+    await user.click(screen.getByRole('button', { name: /Good response/i }));
+
+    expect(screen.getByText('ThumbsUpIcon')).toBeInTheDocument();
+    expect(screen.queryByText('OutlinedThumbsUpIcon')).not.toBeInTheDocument();
+  });
 });

packages/module/src/Message/Message.tsx

@@ -197,6 +197,8 @@ export interface MessageProps extends Omit<HTMLProps<HTMLDivElement>, 'role'> {
   hasNoImagesInUserMessages?: boolean;
   /** Sets background colors to be appropriate on primary chatbot background */
   isPrimary?: boolean;
+  /** When true, automatically swaps to filled icon variants when predefined actions are clicked. */
+  useFilledIconsOnClick?: boolean;
 }
 
 export const MessageBase: FunctionComponent<MessageProps> = ({
@@ -249,6 +251,7 @@ export const MessageBase: FunctionComponent<MessageProps> = ({
   toolCall,
   hasNoImagesInUserMessages = true,
   isPrimary,
+  useFilledIconsOnClick,
   ...props
 }: MessageProps) => {
   const [messageText, setMessageText] = useState(content);
@@ -385,11 +388,16 @@ export const MessageBase: FunctionComponent<MessageProps> = ({
                             key={index}
                             actions={actionGroup.actions || actionGroup}
                             persistActionSelection={persistActionSelection || actionGroup.persistActionSelection}
+                            useFilledIconsOnClick={useFilledIconsOnClick}
                           />
                         ))}
                       </div>
                     ) : (
-                      <ResponseActions actions={actions} persistActionSelection={persistActionSelection} />
+                      <ResponseActions
+                        actions={actions}
+                        persistActionSelection={persistActionSelection}
+                        useFilledIconsOnClick={useFilledIconsOnClick}
+                      />
                     )}
                   </>
                 )}

packages/module/src/ResponseActions/ResponseActions.test.tsx

@@ -5,6 +5,21 @@ import userEvent from '@testing-library/user-event';
 import { DownloadIcon, InfoCircleIcon, RedoIcon } from '@patternfly/react-icons';
 import Message from '../Message';
 
+// Mock the icon components
+jest.mock('@patternfly/react-icons', () => ({
+  OutlinedThumbsUpIcon: () => <div>OutlinedThumbsUpIcon</div>,
+  ThumbsUpIcon: () => <div>ThumbsUpIcon</div>,
+  OutlinedThumbsDownIcon: () => <div>OutlinedThumbsDownIcon</div>,
+  ThumbsDownIcon: () => <div>ThumbsDownIcon</div>,
+  OutlinedCopyIcon: () => <div>OutlinedCopyIcon</div>,
+  DownloadIcon: () => <div>DownloadIcon</div>,
+  InfoCircleIcon: () => <div>InfoCircleIcon</div>,
+  RedoIcon: () => <div>RedoIcon</div>,
+  ExternalLinkAltIcon: () => <div>ExternalLinkAltIcon</div>,
+  VolumeUpIcon: () => <div>VolumeUpIcon</div>,
+  PencilAltIcon: () => <div>PencilAltIcon</div>
+}));
+
 const ALL_ACTIONS = [
   { type: 'positive', label: 'Good response', clickedLabel: 'Good response recorded' },
   { type: 'negative', label: 'Bad response', clickedLabel: 'Bad response recorded' },
@@ -421,4 +436,189 @@ describe('ResponseActions', () => {
     await userEvent.click(customBtn);
     expect(customBtn).not.toHaveClass('pf-chatbot__button--response-action-clicked');
   });
+
+  describe('icon swapping with useFilledIconsOnClick', () => {
+    it('should render outline icons by default', () => {
+      render(
+        <ResponseActions
+          actions={{
+            positive: { onClick: jest.fn() },
+            negative: { onClick: jest.fn() }
+          }}
+        />
+      );
+
+      expect(screen.getByText('OutlinedThumbsUpIcon')).toBeInTheDocument();
+      expect(screen.getByText('OutlinedThumbsDownIcon')).toBeInTheDocument();
+
+      expect(screen.queryByText('ThumbsUpIcon')).not.toBeInTheDocument();
+      expect(screen.queryByText('ThumbsDownIcon')).not.toBeInTheDocument();
+    });
+
+    describe('positive actions', () => {
+      it('should not swap positive icon when clicked and useFilledIconsOnClick is false', async () => {
+        const user = userEvent.setup();
+
+        render(
+          <ResponseActions
+            actions={{
+              positive: { onClick: jest.fn() }
+            }}
+            useFilledIconsOnClick={false}
+          />
+        );
+
+        await user.click(screen.getByRole('button', { name: 'Good response' }));
+
+        expect(screen.getByText('OutlinedThumbsUpIcon')).toBeInTheDocument();
+        expect(screen.queryByText('ThumbsUpIcon')).not.toBeInTheDocument();
+      });
+
+      it('should swap positive icon from outline to filled when clicked with useFilledIconsOnClick', async () => {
+        const user = userEvent.setup();
+
+        render(
+          <ResponseActions
+            actions={{
+              positive: { onClick: jest.fn() }
+            }}
+            useFilledIconsOnClick
+          />
+        );
+
+        await user.click(screen.getByRole('button', { name: 'Good response' }));
+
+        expect(screen.getByText('ThumbsUpIcon')).toBeInTheDocument();
+        expect(screen.queryByText('OutlinedThumbsUpIcon')).not.toBeInTheDocument();
+      });
+
+      it('should revert positive icon to outline icon when clicking outside', async () => {
+        const user = userEvent.setup();
+
+        render(
+          <div>
+            <ResponseActions
+              actions={{
+                positive: { onClick: jest.fn() }
+              }}
+              useFilledIconsOnClick
+            />
+            <div data-testid="outside">Outside</div>
+          </div>
+        );
+
+        await user.click(screen.getByRole('button', { name: 'Good response' }));
+        expect(screen.getByText('ThumbsUpIcon')).toBeInTheDocument();
+
+        await user.click(screen.getByTestId('outside'));
+        expect(screen.getByText('OutlinedThumbsUpIcon')).toBeInTheDocument();
+      });
+
+      it('should not revert positive icon to outline icon when clicking outside if persistActionSelection is true', async () => {
+        const user = userEvent.setup();
+
+        render(
+          <div>
+            <ResponseActions
+              actions={{
+                positive: { onClick: jest.fn() }
+              }}
+              persistActionSelection
+              useFilledIconsOnClick
+            />
+            <div data-testid="outside">Outside</div>
+          </div>
+        );
+
+        await user.click(screen.getByRole('button', { name: 'Good response' }));
+        expect(screen.getByText('ThumbsUpIcon')).toBeInTheDocument();
+
+        await user.click(screen.getByTestId('outside'));
+        expect(screen.getByText('ThumbsUpIcon')).toBeInTheDocument();
+      });
+
+      describe('negative actions', () => {
+        it('should not swap negative icon when clicked and useFilledIconsOnClick is false', async () => {
+          const user = userEvent.setup();
+
+          render(
+            <ResponseActions
+              actions={{
+                negative: { onClick: jest.fn() }
+              }}
+              useFilledIconsOnClick={false}
+            />
+          );
+
+          await user.click(screen.getByRole('button', { name: 'Bad response' }));
+
+          expect(screen.getByText('OutlinedThumbsDownIcon')).toBeInTheDocument();
+          expect(screen.queryByText('ThumbsDownIcon')).not.toBeInTheDocument();
+        });
+
+        it('should swap negative icon from outline to filled when clicked with useFilledIconsOnClick', async () => {
+          const user = userEvent.setup();
+
+          render(
+            <ResponseActions
+              actions={{
+                negative: { onClick: jest.fn() }
+              }}
+              useFilledIconsOnClick
+            />
+          );
+
+          await user.click(screen.getByRole('button', { name: 'Bad response' }));
+
+          expect(screen.getByText('ThumbsDownIcon')).toBeInTheDocument();
+          expect(screen.queryByText('OutlinedThumbsDownIcon')).not.toBeInTheDocument();
+        });
+
+        it('should revert negative icon to outline when clicking outside', async () => {
+          const user = userEvent.setup();
+
+          render(
+            <div>
+              <ResponseActions
+                actions={{
+                  negative: { onClick: jest.fn() }
+                }}
+                useFilledIconsOnClick
+              />
+              <div data-testid="outside">Outside</div>
+            </div>
+          );
+
+          await user.click(screen.getByRole('button', { name: 'Bad response' }));
+          expect(screen.getByText('ThumbsDownIcon')).toBeInTheDocument();
+
+          await user.click(screen.getByTestId('outside'));
+          expect(screen.getByText('OutlinedThumbsDownIcon')).toBeInTheDocument();
+        });
+
+        it('should not revert negative icon to outline icon when clicking outside if persistActionSelection is true', async () => {
+          const user = userEvent.setup();
+
+          render(
+            <div>
+              <ResponseActions
+                actions={{
+                  negative: { onClick: jest.fn() }
+                }}
+                persistActionSelection
+                useFilledIconsOnClick
+              />
+              <div data-testid="outside">Outside</div>
+            </div>
+          );
+
+          await user.click(screen.getByRole('button', { name: 'Bad response' }));
+          expect(screen.getByText('ThumbsDownIcon')).toBeInTheDocument();
+
+          await user.click(screen.getByTestId('outside'));
+          expect(screen.getByText('ThumbsDownIcon')).toBeInTheDocument();
+        });
+      });
+    });
+  });
 });