You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by "richardji202 (via GitHub)" <gi...@apache.org> on 2023/08/19 09:10:09 UTC
[GitHub] [inlong] richardji202 opened a new pull request, #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
richardji202 opened a new pull request, #8770:
URL: https://github.com/apache/inlong/pull/8770
### Prepare a Pull Request
- Fixes #8008
### Motivation
Add default icon for different data nodes
### Modifications
search and add default icon for different data nodes
The CheckCard component supports loading of local logo images
### Verifying this change
*(Please pick either of the following options)*
- [x] This change is a trivial rework/code cleanup without any test coverage.
- [ ] This change is already covered by existing tests, such as:
*(please describe tests)*
- [ ] This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end deployment with large payloads (10MB)*
- *Extended integration test for recovery after broker failure*
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] leezng commented on a diff in pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "leezng (via GitHub)" <gi...@apache.org>.
leezng commented on code in PR #8770:
URL: https://github.com/apache/inlong/pull/8770#discussion_r1299519312
##########
inlong-dashboard/src/ui/components/CheckCard/index.tsx:
##########
@@ -72,8 +106,7 @@ const CheckCard: React.FC<CheckCardProps> = ({ options, value, onChange, disable
bodyStyle={{ textAlign: 'center' }}
className={disabled ? styles.cardDisabled : ''}
>
- <DatabaseOutlined style={{ fontSize: 20 }} />
- <div>{options.find(item => item.value === currentValue)?.label}</div>
+ {renderContent(options.find(item => item.value === currentValue)?.label)}
Review Comment:
Can you add a [tooltip](https://ant.design/components/tooltip)? Some logos have no clear meaning, for example when I hover to `File`.
##########
inlong-dashboard/src/ui/components/CheckCard/index.tsx:
##########
@@ -52,6 +54,25 @@ const CheckCard: React.FC<CheckCardProps> = ({ options, value, onChange, disable
// eslint-disable-next-line
}, [value]);
+ useEffect(() => {
+ // vite 不支持 require.context, 故使用import()动态获取每个类型的logo，模块是否存在由promise判断，模块缓存由esm处理
Review Comment:
Please translate to english.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] richardji202 commented on a diff in pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "richardji202 (via GitHub)" <gi...@apache.org>.
richardji202 commented on code in PR #8770:
URL: https://github.com/apache/inlong/pull/8770#discussion_r1299633179
##########
inlong-dashboard/src/ui/components/CheckCard/logo/Mongodb.png:
##########
Review Comment:
I changed both the name of the PNG image and the label to "MongoDB".
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] richardji202 commented on a diff in pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "richardji202 (via GitHub)" <gi...@apache.org>.
richardji202 commented on code in PR #8770:
URL: https://github.com/apache/inlong/pull/8770#discussion_r1299744644
##########
inlong-dashboard/src/ui/components/CheckCard/logo/Mongodb.png:
##########
Review Comment:
Sorry, it seems that there was inconsistency between the remote repository and my local repository. It has been fixed now.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "healchow (via GitHub)" <gi...@apache.org>.
healchow commented on code in PR #8770:
URL: https://github.com/apache/inlong/pull/8770#discussion_r1299520957
##########
inlong-dashboard/src/ui/components/CheckCard/logo/Mongodb.png:
##########
Review Comment:
Can this file name change to `MongoDB.png`?
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] richardji202 commented on a diff in pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "richardji202 (via GitHub)" <gi...@apache.org>.
richardji202 commented on code in PR #8770:
URL: https://github.com/apache/inlong/pull/8770#discussion_r1299536919
##########
inlong-dashboard/src/ui/components/CheckCard/logo/Mongodb.png:
##########
Review Comment:
The dynamic loading of images is currently based on the "label" of this node, and the "label" of this node is "Mongodb". The configuration file ("@/src/plugins/sources/defaults/Mongodb.ts") is also "Mongodb". Please confirm how to adjust it.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow merged pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "healchow (via GitHub)" <gi...@apache.org>.
healchow merged PR #8770:
URL: https://github.com/apache/inlong/pull/8770
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] dockerzhang commented on pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "dockerzhang (via GitHub)" <gi...@apache.org>.
dockerzhang commented on PR #8770:
URL: https://github.com/apache/inlong/pull/8770#issuecomment-1685211792
@richardji202 thanks for your contribution. Please add a rendering image for this PR.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "healchow (via GitHub)" <gi...@apache.org>.
healchow commented on code in PR #8770:
URL: https://github.com/apache/inlong/pull/8770#discussion_r1299676902
##########
inlong-dashboard/src/ui/components/CheckCard/logo/Mongodb.png:
##########
Review Comment:
Sorry, but why do we need 2 images for MongoDB here?
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #8770: [INLONG-8008][Dashboard] Add default icon for different data nodes
Posted by "healchow (via GitHub)" <gi...@apache.org>.
healchow commented on code in PR #8770:
URL: https://github.com/apache/inlong/pull/8770#discussion_r1299519209
##########
inlong-dashboard/src/ui/components/CheckCard/index.tsx:
##########
@@ -52,6 +54,25 @@ const CheckCard: React.FC<CheckCardProps> = ({ options, value, onChange, disable
// eslint-disable-next-line
}, [value]);
+ useEffect(() => {
+ // vite 不支持 require.context, 故使用import()动态获取每个类型的logo，模块是否存在由promise判断，模块缓存由esm处理
Review Comment:
Please change the comment to English, thanks.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org