Skip to content
Merged

Pm 2179 #1315

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
@import '@libs/ui/styles/includes';

.tableWrapper {
min-height: calc($content-height - 250px);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
The use of a hardcoded value (250px) in the min-height calculation could lead to maintainability issues. Consider defining this value as a variable or constant to improve flexibility and ease of updates.

font-size: 14px;

&:global(.enhanced-table) {
table {
th,
td {
text-align: left;
background-color: white;
}

}
}
}

.tableCell {
vertical-align: middle;
}

.filenameCell {
display: flex;
align-items: center;
gap: 6px;
color: $link-blue-dark;
cursor: pointer;
opacity: 1;
transition: opacity 0.15s ease;

&:hover {
text-decoration: underline;
}
}

.downloading {
cursor: wait;
opacity: 0.6;
}

.expired {
cursor: not-allowed;
color: #999;

&:hover {
text-decoration: none;
}
}

.artifactType {
display: flex;
gap: 5px;
align-items: center;

.artifactIcon {
stroke: #00797A;
}
}

.noAttachmentText {
text-align: center;
}

.mobileRow {
padding: 16px 8px;
border-bottom: 1px solid #A8A8A8;
}

.mobileHeader {
display: flex;
gap: 12px;
}

.mobileExpanded {
padding: 16px 20px 0px 32px;
}

.rowItem {
display: flex;
justify-content: space-between;
}

.rowItemHeading {
font-weight: 700;
color: #0a0a0a;
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,204 @@
import { FC } from 'react'
import { FC, useCallback, useMemo, useState } from 'react'
import { noop } from 'lodash'
import classNames from 'classnames'
import moment from 'moment'

// import styles from './ScorecardAttachments.module.scss'
import { IconOutline, Table, TableColumn } from '~/libs/ui'
import { useReviewsContext } from '~/apps/review/src/pages/reviews/ReviewsContext'
import { useWindowSize, WindowSize } from '~/libs/shared'

import { AiWorkflowRunArtifact,
AiWorkflowRunArtifactDownloadResponse,
AiWorkflowRunAttachmentsResponse,
useDownloadAiWorkflowsRunArtifact, useFetchAiWorkflowsRunAttachments } from '../../../hooks'
import { TableWrapper } from '../../TableWrapper'
import { TABLE_DATE_FORMAT } from '../../../constants'
import { formatFileSize } from '../../common'
import { ReviewsContextModel } from '../../../models'

import styles from './ScorecardAttachments.module.scss'

interface ScorecardAttachmentsProps {
className?: string
}

const ScorecardAttachments: FC<ScorecardAttachmentsProps> = props => (
<div className={props.className}>
attachments
</div>
)
const ScorecardAttachments: FC<ScorecardAttachmentsProps> = (props: ScorecardAttachmentsProps) => {
const className = props.className
const { width: screenWidth }: WindowSize = useWindowSize()
const isTablet = useMemo(() => screenWidth <= 1000, [screenWidth])
const { workflowId, workflowRun }: ReviewsContextModel = useReviewsContext()
const { artifacts }: AiWorkflowRunAttachmentsResponse
= useFetchAiWorkflowsRunAttachments(workflowId, workflowRun?.id)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
Consider adding error handling for the useFetchAiWorkflowsRunAttachments hook to manage cases where the fetch might fail. This will improve the robustness of the component.

const { download, isDownloading }: AiWorkflowRunArtifactDownloadResponse = useDownloadAiWorkflowsRunArtifact(
workflowId,
workflowRun?.id,
)

const handleDownload = useCallback(
async (artifactId: number): Promise<void> => {
await download(artifactId)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The handleDownload function should include error handling to manage potential failures during the download process. This will ensure that the user is informed if something goes wrong.

},
[download],
)

const createDownloadHandler = useCallback(
(id: number) => () => handleDownload(id),
[handleDownload],
)

const columns = useMemo<TableColumn<AiWorkflowRunArtifact>[]>(
() => [
{
className: classNames(styles.tableCell),
label: 'Filename',
propertyName: 'name',
renderer: (attachment: AiWorkflowRunArtifact) => {
const isExpired = attachment.expired

return (
<div
className={classNames(
styles.filenameCell,
{
[styles.expired]: isExpired,
[styles.downloading]: isDownloading && !isExpired,
},
)}
onClick={!isExpired ? createDownloadHandler(attachment.id) : undefined}
>
<span>{attachment.name}</span>
{isExpired && <span>(Link Expired)</span>}
</div>
)
},
type: 'element',
},
{
className: classNames(styles.tableCell),
label: 'Type',
renderer: () => (
<div className={styles.artifactType}>
<IconOutline.CubeIcon className={styles.artifactIcon} width={26} />
<span>Artifact</span>
</div>
),
type: 'element',
},
{
className: classNames(styles.tableCell),
label: 'Size',
propertyName: 'sizeInBytes',
renderer: (attachment: AiWorkflowRunArtifact) => (
<div>{formatFileSize(attachment.size_in_bytes)}</div>
),
type: 'element',
},
{
className: styles.tableCell,
label: 'Attached Date',
renderer: (attachment: AiWorkflowRunArtifact) => (
<span className='last-element'>
{moment(attachment.created_at)
.local()
.format(TABLE_DATE_FORMAT)}
</span>
),
type: 'element',
},
],
[createDownloadHandler, isDownloading],
)

const [openRow, setOpenRow] = useState<number | undefined>(undefined)
const toggleRow = useCallback(
(id: number) => () => {
setOpenRow(prev => (prev === id ? undefined : id))
},
[],
)
const renderMobileRow = (attachment: AiWorkflowRunArtifact): JSX.Element => {
const isExpired = attachment.expired
const downloading = isDownloading
const isOpen = openRow === attachment.id

return (
<div key={attachment.id} className={styles.mobileRow}>
{/* Top collapsed row */}
<div className={styles.mobileHeader}>
<IconOutline.ChevronDownIcon
onClick={toggleRow(attachment.id)}
className={classNames(styles.chevron, {
[styles.open]: isOpen,
})}
width={20}
/>
<div
className={classNames(styles.filenameCell, {
[styles.expired]: isExpired,
[styles.downloading]: downloading,
})}
onClick={!isExpired ? createDownloadHandler(attachment.id) : undefined}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 design]
The onClick handler for downloading an attachment is conditionally set to undefined if the attachment is expired. This is correct, but consider adding a cursor: not-allowed style to visually indicate that the download action is disabled for expired attachments, improving user experience.

>
{attachment.name}
</div>

</div>

{/* Expanded content */}
{isOpen && (
<div className={styles.mobileExpanded}>
<div className={styles.rowItem}>
<span className={styles.rowItemHeading}>Type:</span>
<div className={styles.artifactType}>
<IconOutline.CubeIcon className={styles.artifactIcon} width={20} />
Artifact
</div>
</div>

<div className={styles.rowItem}>
<span className={styles.rowItemHeading}>Size:</span>
{formatFileSize(attachment.size_in_bytes)}
</div>

<div className={styles.rowItem}>
<span className={styles.rowItemHeading}>Date:</span>
{moment(attachment.created_at)
.local()
.format(TABLE_DATE_FORMAT)}
</div>
</div>
)}
</div>
)
}

return (
<TableWrapper
className={classNames(
styles.tableWrapper,
className,
'enhanced-table',
)}
>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 design]
Consider adding a loading state or indicator while the attachments are being fetched. This will enhance the user experience by providing feedback that the data is being loaded.

{!artifacts || artifacts.length === 0 ? (
<div className={styles.noAttachmentText}>No attachments</div>
) : isTablet ? (
<div className={styles.mobileList}>
{artifacts.map(renderMobileRow)}
</div>
) : (
<Table
columns={columns}
data={artifacts}
disableSorting
onToggleSort={noop}
removeDefaultSort
/>
)}

</TableWrapper>
)

}

export default ScorecardAttachments
25 changes: 25 additions & 0 deletions src/apps/review/src/lib/components/common/columnUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,28 @@ export const getProfileUrl = (handle: string): string => {

return `${normalizedBase}/${encodeURIComponent(handle)}`
}

/**
* converts size_in_bytes into KB / MB / GB with correct formatting.
*/
export const formatFileSize = (bytes: number): string => {
if (!bytes || bytes < 0) return '0 B'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The check !bytes will return true for 0, which is not a negative value. Consider changing the condition to bytes <= 0 to handle only negative values and zero correctly.


const KB = 1024
const MB = KB * 1024
const GB = MB * 1024

if (bytes >= GB) {
return `${(bytes / GB).toFixed(2)} GB`
}

if (bytes >= MB) {
return `${(bytes / MB).toFixed(2)} MB`
}

if (bytes >= KB) {
return `${(bytes / KB).toFixed(2)} KB`
}

return `${bytes} B`
}
1 change: 1 addition & 0 deletions src/apps/review/src/lib/constants.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export const SUBMISSION_TYPE_CONTEST = 'CONTEST_SUBMISSION'
export const SUBMISSION_TYPE_CHECKPOINT = 'CHECKPOINT_SUBMISSION'
export const TABLE_DATE_FORMAT = 'MMM DD, HH:mm A'
Loading
Loading