Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new InboxView compound component for call review functionality. The component provides a multi-section interface with a call list, call details with tabbed content, and a call timeline/transcript view.
Changes:
- Added new InboxView compound component with sub-components (CallList, Users, RiskAnalysis, TranscriptItem)
- Enhanced Avatar component with configurable size prop
- Enhanced ContentWrapper component to accept className prop and set height to 100%
- Fixed CSS content property formatting in MenuItem component (space to empty string)
- Reformatted storybook main.cjs for consistency
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/compound/index.ts | Exported new InboxView component and its types |
| lib/compound/InboxView/InboxView.tsx | Main InboxView component with collapsible call list and tabbed interface |
| lib/compound/InboxView/InboxView.types.ts | Type definitions for InboxView (currently empty with TODO) |
| lib/compound/InboxView/index.ts | Export file for InboxView component |
| lib/compound/InboxView/InboxView.stories.tsx | Storybook story for InboxView |
| lib/compound/InboxView/InboxView.module.css | Styles for InboxView component |
| lib/compound/InboxView/parts/CallList.tsx | Call list component with mock data |
| lib/compound/InboxView/parts/Users.tsx | Users tab content placeholder |
| lib/compound/InboxView/parts/RiskAnalysis.tsx | Risk analysis tab content placeholder |
| lib/compound/InboxView/parts/TranscriptItem.tsx | Individual transcript item component |
| lib/atom/Avatar/Avatar.tsx | Added size prop with default value |
| lib/atom/Avatar/types.ts | Added size prop type definition |
| lib/atom/ContentWrapper/ContentWrapper.tsx | Added className prop support and height styling |
| lib/atom/ContentWrapper/ContentWrapper.styles.module.css | Added unused .content class |
| lib/atom/MenuItem/MenuItem.module.css | Fixed CSS content property from space to empty string |
| .storybook/main.cjs | Reformatted getAbsolutePath function to single line |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (6)
lib/compound/InboxView/InboxView.types.ts:13
- The properties in IInboxViewProps should use the
readonlymodifier to be consistent with other compound components in the codebase. All other compound component interfaces (ITableProps, IDateRangePickerProps, ITopbarProps, ISidebarMenu) consistently usereadonlyfor their properties.
content: React.ReactNode;
callList: Array<CallListItem>;
lib/compound/InboxView/InboxView.module.css:47
- The CSS classes
transactionTimelineWrapper,alertDetails, andtabContentRootare defined but never used in any component files. These should either be removed or the component implementation is incomplete.
.transactionTimelineWrapper {
> div {
padding: 0;
}
}
.recentAlerts {
box-shadow: inset -1px 0 0 0 var(--gray-a5);
}
.alertDetails {
background-color: var(--color-background);
z-index: 1;
}
.callListItem {
border-radius: var(--radius-5);
background-color: var(--color-background);
box-shadow: inset 0 0 0 1px var(--gray-a4);
height: fit-content;
width: 100%;
align-items: start;
}
.callListItemHeading {
color: var(--gray-12);
width: 100%;
}
.callListItemSelected {
box-shadow: inset 0 0 0 1px var(--blue-9);
}
.newMessageBadge {
border-radius: var(--radius-5);
background-color: var(--blue-5);
width: 6px;
height: 6px;
}
.tabContentRoot {
height: 100%;
flex: 1;
position: relative;
padding-inline: var(--space-4);
}
lib/compound/InboxView/InboxView.tsx:44
- The
positionproperty is set twice: once through the Flex props API (line 29) and again in the inline style object (line 35). This is redundant and the inline style will override the prop. Remove the duplicate declaration.
<Flex
justify="between"
position="sticky"
top={'0'}
pt="4"
px="4"
pb="2"
style={{
position: 'sticky',
top: 0,
backgroundColor: 'var(--gray-1)',
zIndex: 1,
paddingTop: 'var(--space-4)',
paddingBottom: 'var(--space-2)',
boxShadow: 'inset 0 -1px 0 0 var(--gray-a4)',
paddingInline: 'var(--space-4)',
marginRight: '1px',
}}
lib/atom/ContentWrapper/ContentWrapper.styles.module.css:9
- The CSS class
.contentis defined but never used anywhere in the component. This should be removed to avoid confusion and keep the codebase clean.
.content {
height: 100%;
}
lib/compound/InboxView/parts/CallList.tsx:49
- The aria-description attribute has inverted logic. It should be set to 'New Message' when the message is UNREAD (read is false), but the condition checks for
item.readwhich means it shows the description for read messages instead of unread ones. The condition should be!item.read.
aria-description={item.read ? 'New Message' : undefined}
lib/compound/InboxView/index.ts:2
- The PR description mentions sub-components "Users" and "RiskAnalysis", but these components do not exist in the implementation. Only CallList and TranscriptItem are actually implemented. This discrepancy suggests either incomplete implementation or inaccurate documentation.
export { default } from './InboxView';
export type { IInboxViewProps } from './InboxView.types';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tabs: [ | ||
| { | ||
| label: 'Transcript', | ||
| value: 'transcript', | ||
| content: ( | ||
| <> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| <TranscriptItem /> | ||
| </> | ||
| ), | ||
| }, | ||
| { | ||
| label: 'Details', | ||
| value: 'details', | ||
| content: ( | ||
| <Box> | ||
| <Text size="2" color="gray"> | ||
| Meeting Details: | ||
| </Text> | ||
| <Text size="3">- Date: January 1, 2024</Text> | ||
| <Text size="3">- Time: 12:00 PM</Text> | ||
| <Text size="3">- Participants: Alice, Bob</Text> | ||
| </Box> | ||
| ), | ||
| }, | ||
| ], |
There was a problem hiding this comment.
The story is using a tabs prop that doesn't exist in IInboxViewProps. According to InboxView.types.ts, the component expects content and callList props, but this story is providing tabs instead. Additionally, the required callList prop is completely missing from the story args. This will cause the story to fail or not render properly.
| tabs: [ | |
| { | |
| label: 'Transcript', | |
| value: 'transcript', | |
| content: ( | |
| <> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| </> | |
| ), | |
| }, | |
| { | |
| label: 'Details', | |
| value: 'details', | |
| content: ( | |
| <Box> | |
| <Text size="2" color="gray"> | |
| Meeting Details: | |
| </Text> | |
| <Text size="3">- Date: January 1, 2024</Text> | |
| <Text size="3">- Time: 12:00 PM</Text> | |
| <Text size="3">- Participants: Alice, Bob</Text> | |
| </Box> | |
| ), | |
| }, | |
| ], | |
| content: ( | |
| <> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| <TranscriptItem /> | |
| </> | |
| ), | |
| callList: ( | |
| <Box> | |
| <Text size="2" color="gray"> | |
| Meeting Details: | |
| </Text> | |
| <Text size="3">- Date: January 1, 2024</Text> | |
| <Text size="3">- Time: 12:00 PM</Text> | |
| <Text size="3">- Participants: Alice, Bob</Text> | |
| </Box> | |
| ), |
This PR introduces a new InboxView compound component for call review functionality. The component provides a multi-section interface with a call list, call details with tabbed content, and a call timeline/transcript view.
Changes: