Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(comp:icon): update icons, add more and exit icon #1468

Merged
merged 1 commit into from
Feb 23, 2023
Merged

fix(comp:icon): update icons, add more and exit icon #1468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/components/icon/demo/all.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export const allIcons = [
'exception',
'exclamation-circle-filled',
'exclamation-circle',
'exit',
'expand',
'export',
'eye-invisible',
Expand Down Expand Up @@ -137,6 +138,7 @@ export const allIcons = [
'mirror',
'mobile',
'monitor',
'more',
'move-file',
'move-group',
'move-list',
Copy link

Choose a reason for hiding this comment

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

the code review:

First, it looks like the code patch is adding an additional icon to the list of icons at lines 70 and 138. The new icon being added is 'exit' and 'more'. It appears that the code is properly formatted and follows the correct syntax.

However, there are a few things we can look at to make sure that this code is as efficient as possible. First, we can check to see if the new icons are necessary, as they may already be in the list. Second, we can make sure that the new icons are in alphabetical order so that the list is easier to read and navigate. Third, we can check to make sure that the new icons are correctly spelled and formatted. Finally, we can check to make sure that the new icons are compatible with any existing code that may be using the list of icons.

Overall, it looks like the code patch is properly formatted and follows the correct syntax. However, it would be wise to do a few checks to make sure that the new icons are necessary and compatible with any existing code.

Expand Down
28 changes: 19 additions & 9 deletions packages/components/icon/src/definitions.ts
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/desktop.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/dialog-close.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/down.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/ellipsis.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions scripts/gulp/icons/assets/exit.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/eye-invisible.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/folder.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions scripts/gulp/icons/assets/more.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/plus.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/search.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion scripts/gulp/icons/assets/star.svg
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.