feat(cc-widgets): added new incomingtask and task ui based on figma design #402
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe pull request introduces several changes across multiple packages. In the root configuration, a new runtime dependency (Luxon) and its TypeScript type definitions have been added to the project. Several package.json files in different packages were updated with only newline additions at the end of the files. Significant refactoring occurred in the task-related components: the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
cmullenx
changed the title
cmullenx
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/contact-center/task/src/Task/styles.scss (1)
16-17: Consider avoiding!importantfor height override.While the comment explains why you're using
!important, this approach can lead to specificity issues and make styles harder to override when needed. Consider using more specific selectors or restructuring the CSS to avoid this.- height: 80px !important; + height: 80px;If there's a conflicting style from the Momentum framework, you might want to use a more specific selector to override it instead of using
!important.packages/contact-center/task/src/TaskTimer/index.tsx (1)
30-62: Simplify effect dependencies and optimize timer logic.Your useEffect has
runningin the dependency array which could cause unexpected re-renders. Also, the timer cleanup can be simplified.- useEffect(() => { - if (!running) { - return; - } - - const updateDuration = () => { + useEffect(() => { + if (!running) { + return; + } + + let timeoutId: NodeJS.Timeout; + + const updateDuration = () => { const now = DateTime.utc(); let diff: Duration; if (countdown && ronaTimeout !== undefined) { const end = start.plus({seconds: ronaTimeout}); diff = end.diff(now); if (diff.toMillis() <= 0) { setDuration('00:00'); setRunning(false); return; } } else { diff = now.diff(start); } setDuration(formatDuration(diff)); - const timeoutId = setTimeout(updateDuration, 1000); - - return () => clearTimeout(timeoutId); + timeoutId = setTimeout(updateDuration, 1000); }; updateDuration(); - return () => setRunning(false); // Ensure running state is cleaned up - }, [countdown, ronaTimeout, startTimeStamp, running]); + return () => { + clearTimeout(timeoutId); + setRunning(false); // Ensure running state is cleaned up + }; + }, [countdown, ronaTimeout, startTimeStamp, running, start]);Also, note that I've added
startto the dependency array since it's used in the effect.packages/contact-center/task/src/Task/index.tsx (3)
33-35: Move utility function outside component.The
capitalizeFirstWordfunction is recreated on every render. Consider moving it outside the component to improve performance.+ const capitalizeFirstWord = (str: string) => { + return str.replace(/^\s*(\w)/, (match, firstLetter) => firstLetter.toUpperCase()); + }; + const Task: React.FC<TaskProps> = ({ title, state, startTimeStamp, ronaTimeout, selected = false, isIncomingTask = false, queue, acceptTask, declineTask, isBrowser, }) => { - const capitalizeFirstWord = (str: string) => { - return str.replace(/^\s*(\w)/, (match, firstLetter) => firstLetter.toUpperCase()); - };
66-74: Simplify conditional rendering logic.The current conditional expression is complex and could be simplified for better readability.
- {/* Handle Time should render if it's an incoming call without ronaTimeout OR if it's not an incoming call */} - {(isIncomingTask && !ronaTimeout) || !isIncomingTask - ? startTimeStamp && ( - <Text tagName="span" type="body-midsize-regular" className="task-text task-text--secondary"> - Handle Time: {' '} - <TaskTimer startTimeStamp={startTimeStamp} /> - </Text> - ) - : null} + {/* Handle Time should render if it's an incoming call without ronaTimeout OR if it's not an incoming call */} + {((!isIncomingTask || !ronaTimeout) && startTimeStamp) && ( + <Text tagName="span" type="body-midsize-regular" className="task-text task-text--secondary"> + Handle Time: {' '} + <TaskTimer startTimeStamp={startTimeStamp} /> + </Text> + )}
86-96: Improve button rendering logic.The conditional logic for displaying buttons could be simplified.
<ListItemBaseSection position="end"> - {isIncomingTask ? ( - <ButtonPill onPress={acceptTask} color="join" disabled={!isBrowser}> - Ringing - </ButtonPill> - ) : isBrowser ? ( - <ButtonPill onPress={declineTask} color="join"> - End - </ButtonPill> - ) : null} + {isIncomingTask && ( + <ButtonPill onPress={acceptTask} color="join" disabled={!isBrowser}> + Ringing + </ButtonPill> + )} + {!isIncomingTask && isBrowser && ( + <ButtonPill onPress={declineTask} color="join"> + End + </ButtonPill> + )} </ListItemBaseSection>packages/contact-center/task/src/TaskList/task-list.presentational.tsx (1)
12-35: Verify presence of callAssociatedDetails and consider using stable keys.
- If
callAssociatedDetailsmight be missing, destructuring it will cause a runtime error. Verify it’s always present, or add a fallback:- const {ani, virtualTeamName, ronaTimeout} = task.data.interaction.callAssociatedDetails; + const {ani, virtualTeamName, ronaTimeout} = + task.data.interaction.callAssociatedDetails || {};
- Using the array index as a React key can lead to issues with item reordering. If
interactionId(or another stable ID) is guaranteed unique, consider using it:- key={index} + key={task.data.interactionId}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
package.json(2 hunks)packages/contact-center/cc-components/package.json(1 hunks)packages/contact-center/cc-widgets/package.json(1 hunks)packages/contact-center/station-login/package.json(1 hunks)packages/contact-center/store/package.json(1 hunks)packages/contact-center/task/package.json(1 hunks)packages/contact-center/task/src/IncomingTask/incoming-task.presentational.tsx(1 hunks)packages/contact-center/task/src/Task/index.tsx(1 hunks)packages/contact-center/task/src/Task/styles.scss(1 hunks)packages/contact-center/task/src/TaskList/styles.scss(1 hunks)packages/contact-center/task/src/TaskList/task-list.presentational.tsx(1 hunks)packages/contact-center/task/src/TaskTimer/index.tsx(1 hunks)packages/contact-center/user-state/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- packages/contact-center/user-state/package.json
- packages/contact-center/task/package.json
- packages/contact-center/cc-widgets/package.json
- packages/contact-center/station-login/package.json
- packages/contact-center/task/src/TaskList/styles.scss
- packages/contact-center/cc-components/package.json
- packages/contact-center/store/package.json
🔇 Additional comments (8)
package.json (2)
20-20: Add Luxon TypeScript Definitions
The addition of@types/luxonin thedevDependenciessection improves type safety for Luxon usage in TypeScript. Ensure that the version (^3) aligns with the rest of the project's TypeScript definitions.
66-68: Include Luxon Runtime Dependency
The new dependency"luxon": "^3.5.0"in thedependenciessection provides robust date and time utilities required by the new feature. Verify that this version is compatible with other packages and meets design expectations.packages/contact-center/task/src/Task/styles.scss (1)
1-9: LGTM! Great support for light and dark mode.The CSS custom property implementation with media query for dark mode is a clean approach to handle theme variations.
packages/contact-center/task/src/TaskTimer/index.tsx (1)
4-7: Improve formatDuration to handle edge cases.The current implementation handles negative values correctly, but it could be more robust with additional checks.
The approach to prevent negative values and format based on the duration length is good practice.
packages/contact-center/task/src/Task/index.tsx (1)
8-19: Consider required props instead of optional.Most props are marked as optional, but some might be required for proper rendering.
You've made all props optional which could lead to unexpected rendering states. Consider making critical props like
titlerequired if they're essential for proper component functionality. It's also helpful to add prop validation for required relationships (e.g., ifisIncomingTaskis true, thenqueueshould be present).packages/contact-center/task/src/IncomingTask/incoming-task.presentational.tsx (1)
16-25: LGTM! Great modularization with the new Task component.The refactoring to use the new Task component simplifies the code and improves maintainability by encapsulating the task display logic.
packages/contact-center/task/src/TaskList/task-list.presentational.tsx (2)
3-4: Imports look clean and organized.
No issues found with addingTaskand the external stylesheet.
7-8: Short-circuiting when the task list is empty is appropriate.
Returning an empty fragment avoids unnecessary rendering of markup.
packages/contact-center/task/src/IncomingTask/incoming-task.presentational.tsx
Outdated
Show resolved
Hide resolved
Shreyas281299
changed the title
Shreyas281299
changed the title
…script, use rems instead of pixels
fix(task): remove-ui-testing
|
🎉 This PR is included in version 1.28.0-ccwidgets.26 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.28.0-ccwidgets.26](webex/widgets@v1.28.0-ccwidgets.25...v1.28.0-ccwidgets.26) (2025-03-03) ### Features * **cc-widgets:** added new incomingtask and task ui based on figma design ([webex#402](webex#402)) ([e69896a](webex@e69896a))
COMPLETES # https://jira-eng-sjc12.cisco.com/jira/browse/CAI-6035
This pull request addresses
UI for Task and IncomingTask
by making the following changes
Non WebRTC UI
WebRTC UI
Multi Tabs
Change Type
The following scenarios were tested
Manually testing the UI locally
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.