Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
19 changes: 19 additions & 0 deletions modules/web/src/component/Dialog/DeploymentDetailDialog/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
import { Deployment } from '@/types/deployment';
import { Pod } from '@/types/pod';
import YAMLViewerDialog from '../YAMLViewerDialog';
import { useAlert } from '@/hook/useAlert';
import { copyToClipboard } from '@/helper/util';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The copyToClipboard function is imported from @/helper/util, but it doesn't appear to be exported from modules/web/src/helper/util.ts. This will cause a build failure. Please ensure this utility function is correctly implemented and exported from the specified file.


interface TabPanelProps {
children?: React.ReactNode;
Expand Down Expand Up @@ -52,6 +54,7 @@ interface DeploymentDetailDialogProps {
function DeploymentDetailDialog({ open, onClose, data, pods }: DeploymentDetailDialogProps) {
const [tab, setTab] = React.useState(0);
const [yamlDialogOpen, setYamlDialogOpen] = React.useState(false);
const { success } = useAlert();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To provide feedback to the user when a copy action fails, you should also retrieve the error function from useAlert. This will be used in the error handling for the copy functions.

Suggested change
const { success } = useAlert();
const { success, error } = useAlert();


const handleYamlOpen = () => {
setYamlDialogOpen(true);
Expand All @@ -63,6 +66,20 @@ function DeploymentDetailDialog({ open, onClose, data, pods }: DeploymentDetailD

const displayPods = pods?.filter((pod) => pod.metadata?.ownerReferences?.[0]?.name?.includes(data?.metadata?.name || '')) || [];

const handleCopyName = async () => {
if (data?.metadata?.name) {
await copyToClipboard(String(data.metadata.name));
success('Copied name');
}
};

const handleCopyUID = async () => {
if (data?.metadata?.uid) {
await copyToClipboard(String(data.metadata.uid));
success('Copied ID');
}
};
Comment on lines +66 to +78

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The handleCopyName and handleCopyUID functions lack error handling for the copyToClipboard async operation, which can lead to unhandled promise rejections. Also, the String() conversion is unnecessary.

This suggestion adds try...catch blocks for robust error handling. For further improvement, consider refactoring these two similar functions into a single generic handler to reduce code duplication.

  const handleCopyName = async () => {
    if (data?.metadata?.name) {
      try {
        await copyToClipboard(data.metadata.name);
        success('Copied name');
      } catch (err) {
        console.error('Failed to copy name:', err);
        error('Failed to copy name');
      }
    }
  };

  const handleCopyUID = async () => {
    if (data?.metadata?.uid) {
      try {
        await copyToClipboard(data.metadata.uid);
        success('Copied ID');
      } catch (err) {
        console.error('Failed to copy ID:', err);
        error('Failed to copy ID');
      }
    }
  };


return (
<>
<Dialog open={!!open} onClose={onClose} maxWidth="md" fullWidth>
Expand Down Expand Up @@ -208,6 +225,8 @@ function DeploymentDetailDialog({ open, onClose, data, pods }: DeploymentDetailD
</DialogContent>
<DialogActions>
<Button onClick={onClose}>Cancel</Button>
<Button onClick={handleCopyName} variant="outlined">Copy Name</Button>
<Button onClick={handleCopyUID} variant="outlined">Copy ID</Button>
<Button onClick={handleYamlOpen} variant="contained">
YAML
</Button>
Expand Down
19 changes: 19 additions & 0 deletions modules/web/src/component/Dialog/NodeDetailDialog/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React, { useState } from 'react';
import { Box, Button, Dialog, DialogTitle, DialogContent, DialogActions, Typography } from '@mui/material';
import { useAlert } from '@/hook/useAlert';
import { copyToClipboard } from '@/helper/util';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The copyToClipboard function is imported from @/helper/util, but it doesn't appear to be exported from modules/web/src/helper/util.ts. This will cause a build failure. Please ensure this utility function is correctly implemented and exported from the specified file.

import { useTheme } from '@mui/material/styles';
import YAMLViewerDialog from '@/component/Dialog/YAMLViewerDialog';
import { Node } from '@/types/node';
Expand All @@ -15,6 +17,7 @@ interface NodeDetailDialogProps {
export function NodeDetailDialog({ open, onClose, data }: NodeDetailDialogProps) {
const [yamlDialogOpen, setYamlDialogOpen] = useState(false);
const theme = useTheme();
const { success } = useAlert();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To provide feedback to the user when a copy action fails, you should also retrieve the error function from useAlert. This will be used in the error handling for the copy functions.

Suggested change
const { success } = useAlert();
const { success, error } = useAlert();


const handleYamlOpen = () => {
setYamlDialogOpen(true);
Expand All @@ -24,6 +27,20 @@ export function NodeDetailDialog({ open, onClose, data }: NodeDetailDialogProps)
setYamlDialogOpen(false);
};

const handleCopyName = async () => {
if (data?.metadata?.name) {
await copyToClipboard(String(data.metadata.name));
success('Copied name');
}
};

const handleCopyUID = async () => {
if (data?.metadata?.uid) {
await copyToClipboard(String(data.metadata.uid));
success('Copied ID');
}
};
Comment on lines +29 to +41

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The handleCopyName and handleCopyUID functions lack error handling for the copyToClipboard async operation, which can lead to unhandled promise rejections. Also, the String() conversion is unnecessary.

This suggestion adds try...catch blocks for robust error handling. For further improvement, consider refactoring these two similar functions into a single generic handler to reduce code duplication.

  const handleCopyName = async () => {
    if (data?.metadata?.name) {
      try {
        await copyToClipboard(data.metadata.name);
        success('Copied name');
      } catch (err) {
        console.error('Failed to copy name:', err);
        error('Failed to copy name');
      }
    }
  };

  const handleCopyUID = async () => {
    if (data?.metadata?.uid) {
      try {
        await copyToClipboard(data.metadata.uid);
        success('Copied ID');
      } catch (err) {
        console.error('Failed to copy ID:', err);
        error('Failed to copy ID');
      }
    }
  };


return (
<>
<Dialog open={!!open} onClose={onClose} fullWidth maxWidth="md">
Expand Down Expand Up @@ -100,6 +117,8 @@ export function NodeDetailDialog({ open, onClose, data }: NodeDetailDialogProps)
</DialogContent>
<DialogActions>
<Button onClick={onClose}>Cancel</Button>
<Button onClick={handleCopyName} variant="outlined">Copy Name</Button>
<Button onClick={handleCopyUID} variant="outlined">Copy ID</Button>
<Button onClick={handleYamlOpen} variant="contained">
YAML
</Button>
Expand Down