leezng commented on code in PR #8756:
URL: https://github.com/apache/inlong/pull/8756#discussion_r1297008269


##########
inlong-dashboard/src/ui/components/FieldList/index.tsx:
##########
@@ -30,17 +30,18 @@ export interface Props {
   inlongStreamId?: string;
   isSource: boolean;
   columns: ColumnsType;
-  //   readonly?: string;
 }
 
 const Comp: React.FC<Props> = ({ inlongGroupId, inlongStreamId, isSource, 
columns }) => {
   const [createModal, setCreateModal] = useState<Record<string, unknown>>({
     open: false,
   });
 
+  const [blur, setBlur] = useState(false);

Review Comment:
   I think the newly added `blur` attribute is unnecessary. Can the existing 
props be used to complete the function?
   
   On the other hand, passing `blur` to Modal feels meaningless.



##########
inlong-dashboard/src/ui/components/FieldList/DetailModal.tsx:
##########
@@ -36,6 +36,7 @@ export interface Props extends ModalProps {
   inlongStreamId: string;
   defaultType?: string;

Review Comment:
   Are these properties(id, defaultType, ...) unused?



##########
inlong-dashboard/src/ui/components/FieldList/DetailModal.tsx:
##########
@@ -226,6 +223,12 @@ const Comp: React.FC<Props> = ({
     message.success(t('pages.GroupDetail.Sources.SaveSuccessfully'));
   };
 
+  useEffect(() => {

Review Comment:
   Can the logic of calling `getData` in several `useEffect` here be processed 
in one place? (Three `getData` calls were found) Otherwise, under complex 
conditions, it is easy to cause `getData` in multiple places to be executed, 
leaving hidden dangers.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to