-
Notifications
You must be signed in to change notification settings - Fork 62
feat(task): Update Outdial Dialpad UI to Match Figma #538
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
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
cmullenx
left a comment
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.
what happens if you fail to fetch the outdial ani list? what happens if the call fails? Tests?
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.style.scss
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.style.scss
Outdated
Show resolved
Hide resolved
mkesavan13
left a comment
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 looks like the sync from
ccwidgetsbranch did not happen properly and I see those changes appearing in this PR also - Let us bring back the existing PR description template and fill it in. It is a leadership ask to have the GAI policy as part of all the PRs and filled in appropriately.
- Kindly record a vidcast of the changes to showcase and attach it to the description
- Add a brief note of all the scenarios that were tested
|
vidcast showing functionality: https://app.vidcast.io/share/d0be837f-11f2-4289-ac7c-73d262a78c33 |
mkesavan13
left a comment
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.
- I see neither the enumerated tests nor the vidcast showing if a task actually came in
- The out dial in the Agent Desktop always is auto answered. Did we check for whether it was auto accepted and put in the task list in the widgets?
- Did we try to test this scenario in multi-login scenarios? Epic is heavily dependent on multi-login and it is essential to test that
- w.r.t the components UT for out dial, we do not have the util methods like
regExForDnSpecialChars,validateOutboundNumber,handleOnClicktested. Which is why in other components we have split them into a separate util file to test it separately. We should do the same here
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
| import { | ||
| OutdialAniEntriesResponse, | ||
| OutdialAniParams, | ||
| } from 'node_modules/@webex/contact-center/dist/types/services/config/types'; |
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.
The right way to do this would be to export the type in SDK and then bring it. Pulling it in from node_modules would affect if tomorrow we change the directory structure in the SDK
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.
@mkesavan13 I apologize i was following the example of the other imports in the file. These are indeed being exported by the sdk as seen in Christina's PR, yet when I was trying to import them I was seeing no exported type was found
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.
Oh, I see.
@Shreyas281299 - Could you please check why aren't we able to import directly? Also why do we have other node_modules based imports there?
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.
Il check this.
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
|
@mkesavan13
These functions are being unit tested by the tests that change the inputs and simulate clicks. They can be moved to a utils file if that is your preference |
Did we try and fail the tests to see if the test to read the screen for errors is right? If so, we are good there. @Shreyas281299 - I would like for you to verify this also once |
|
@mkesavan13 after further reviewing the functionality you suggested could be moved to a utils I don't believe it would make sense to move them.
They are both still tested by existing unit test simulations. Please let me know if you have additional thoughts. |
| ); | ||
|
|
||
| // useEffect and useState to allow for async fetching of outdial ANI entries | ||
| useEffect(() => { |
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.
What about pulling entries for address book ? When the outdial component loads, we have 3 options in Agent desktop currently: Dialpad, searching from address book and organization search so since address book is available now would be good to add that as well here
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.
Address book is not part of the scope of my current story, that can be added at a later date. This PR is to update the Outdial Dialpad
| const result = await getOutdialANIEntries(); | ||
| setOutdialANIList(result); | ||
| } catch (error) { | ||
| logger?.error('Error fetching outdial ANI entries:', error); |
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.
Why is logger optional ? This is coming from store and it seems from other components where this is being used, this is not an optional parameter so we can avoid ? here.
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.
@Kesari3008 The optional chaining on logger is required as it will cause unit tests to fail and cause warnings of logger being undefined when running the application otherwise. Logger is not always immediately initialized on first render and throws TypeError: Cannot read properties of undefined in unit tests even when mocked.
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/task.types.ts
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/OutdialCall/outdial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/task.types.ts
Outdated
Show resolved
Hide resolved
| agentConfig?: { | ||
| regexUS: RegExp | string; | ||
| agentId: string; | ||
| outdialANIId: string; |
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.
We will need another value to be read from agentConfig which Outdial flag: isOutboundEnabledForAgent. If outdial is enabled from control hub, this value is received and outdial component should be disabled if this flag is set to false
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.
the ability to show/hide this widget will be in our epic project and not in widgets
| <section className="section-box"> | ||
| <fieldset className="fieldset"> | ||
| <legend className="legend-box">Outdial Call</legend> | ||
| <OutdialCall /> |
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.
Hide this component behind the feature flag mentioned for agentConfig
|
Please add a vidcast for the testing done in PR description. |
packages/contact-center/cc-components/tests/components/task/OutdialCall/out-dial-call.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/tests/components/task/OutdialCall/out-dial-call.tsx
Show resolved
Hide resolved
| import { | ||
| OutdialAniEntriesResponse, | ||
| OutdialAniParams, | ||
| } from 'node_modules/@webex/contact-center/dist/types/services/config/types'; |
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.
Il check this.
| const agentProfile = cc.agentConfig; | ||
| const outdialANIId = agentProfile?.outdialANIId; | ||
| if (!outdialANIId) { | ||
| throw Error('No OutdialANI Id received.'); |
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.
We should not be doing this. If this error is ever triggered it will break the component, I think we should just be logging this as this would be a widget's issue. What do you think?
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.
Sure, I can change it to a logging statement so it does not throw a breaking error
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.
Looking at the sdk changes, I believe calling getOutdialAniEntries without a defined outdialANI will throw an error, which would still break the component right? Is it worth logging the error and letting that happen instead of throwing the error prior to the call?
|
Vidcast showcasing multi-login and desktop call: https://app.vidcast.io/share/be4ad199-98ce-40a7-9007-c70bc4a85e93 |
...es/contact-center/cc-components/tests/components/task/OutdialCall/out-dial-call.snapshot.tsx
Show resolved
Hide resolved
packages/contact-center/cc-components/tests/components/task/OutdialCall/out-dial-call.tsx
Show resolved
Hide resolved
...es/contact-center/cc-components/tests/components/task/OutdialCall/out-dial-call.snapshot.tsx
Show resolved
Hide resolved
# [1.28.0-ccwidgets.119](v1.28.0-ccwidgets.118...v1.28.0-ccwidgets.119) (2025-10-22) ### Features * **task:** Update Outdial Dialpad UI to Match Figma ([#538](#538)) ([07577f6](07577f6))
|
🎉 This PR is included in version 1.28.0-ccwidgets.119 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
COMPLETES # https://jira-eng-sjc12.cisco.com/jira/browse/CAI-7173
This pull request addresses
The outdial component currently does not have full functionality nor does it match the figma, and this PR addresses those areas
by making the following changes
Adding CSS to match the figma, adding the outdial ani list for the dropdown, fixing the unit tests
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The OutdialCallComponent:
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.