-
Notifications
You must be signed in to change notification settings - Fork 212
@W-20975885 New launch agent location in header component #3606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2081fa1
c16ea45
f5d1bc2
cabde5a
8ddbd00
c53deb0
0a1f5eb
58f960d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,20 @@ | ||||||
| /* | ||||||
| * Copyright (c) 2025, Salesforce, Inc. | ||||||
| * All rights reserved. | ||||||
| * SPDX-License-Identifier: BSD-3-Clause | ||||||
| * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||||||
| */ | ||||||
| import {useCallback} from 'react' | ||||||
| import {launchChat} from '@salesforce/retail-react-app/app/utils/shopper-agent-utils' | ||||||
|
|
||||||
| /** | ||||||
| * React hook that returns shopper agent actions. | ||||||
| * Uses the embedded service bootstrap API. Structured for future extension (e.g. close, sendMessage). | ||||||
| */ | ||||||
| export function useShopperAgent() { | ||||||
| const open = useCallback(() => { | ||||||
| launchChat() | ||||||
| }, []) | ||||||
|
|
||||||
| return {actions: {open}} | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you need the key "actions" because it's already an extensible object.
Suggested change
This way you can directly deconstruct it, usage example const {open} = useShopperAgent()i'm just nitpicking.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to keep the actions wrapper to add explicit namespace for executable functions. What you think? |
||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| /* | ||
| * Copyright (c) 2025, Salesforce, Inc. | ||
| * All rights reserved. | ||
| * SPDX-License-Identifier: BSD-3-Clause | ||
| * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||
| */ | ||
|
|
||
| import {renderHook, act} from '@testing-library/react' | ||
| import {useShopperAgent} from '@salesforce/retail-react-app/app/hooks/use-shopper-agent' | ||
| import {launchChat} from '@salesforce/retail-react-app/app/utils/shopper-agent-utils' | ||
|
|
||
| jest.mock('@salesforce/retail-react-app/app/utils/shopper-agent-utils', () => ({ | ||
| launchChat: jest.fn() | ||
| })) | ||
|
|
||
| describe('useShopperAgent', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks() | ||
| }) | ||
|
|
||
| test('should return an object with actions', () => { | ||
| const {result} = renderHook(() => useShopperAgent()) | ||
|
|
||
| expect(result.current).toEqual({actions: expect.any(Object)}) | ||
| expect(result.current.actions).toHaveProperty('open') | ||
| expect(typeof result.current.actions.open).toBe('function') | ||
| }) | ||
|
|
||
| test('should call launchChat when actions.open is invoked', () => { | ||
| const {result} = renderHook(() => useShopperAgent()) | ||
|
|
||
| act(() => { | ||
| result.current.actions.open() | ||
| }) | ||
|
|
||
| expect(launchChat).toHaveBeenCalledTimes(1) | ||
| }) | ||
|
|
||
| test('should call launchChat each time actions.open is invoked', () => { | ||
| const {result} = renderHook(() => useShopperAgent()) | ||
|
|
||
| act(() => { | ||
| result.current.actions.open() | ||
| result.current.actions.open() | ||
| }) | ||
|
|
||
| expect(launchChat).toHaveBeenCalledTimes(2) | ||
| }) | ||
|
|
||
| test('should return a stable open callback reference across re-renders', () => { | ||
| const {result, rerender} = renderHook(() => useShopperAgent()) | ||
|
|
||
| const firstOpen = result.current.actions.open | ||
| rerender() | ||
| const secondOpen = result.current.actions.open | ||
|
|
||
| expect(firstOpen).toBe(secondOpen) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,8 @@ export const getCommerceAgentConfig = () => { | |
| commerceOrgId: '', | ||
| siteId: '', | ||
| enableConversationContext: 'false', | ||
| conversationContext: [] | ||
| conversationContext: [], | ||
| enableAgentFromHeader: 'false' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any idea why the booleans are implemented as strings? I understand this is already there before your change but I'm curious to know if you happen to know the answer.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aggreed its cleaner to use boolean, I'm not sure why the API/Config design uses strings for flags. this is the official help doc Shopper Agent documentation. |
||
| } | ||
| return getConfig().app.commerceAgent ?? defaults | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears the code style is slightly inconsistent, most of the component access the global config object via
getConfig().xxxxxx, and it seems shopper agent introduced its own utilitygetCommerceAgentConfig. I wonder if we want to be consistent, either create utilities for all other features (not just shopper agent), otherwise it looks inconsistent coding styleThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a good point, this is where they decided to change it into utility functions, #3230