-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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.
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
[Icons] Update CSSStyle and add TableSync #6913
Conversation
FormatBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
resources/icons/20px/data-sync.svg
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added table-sync
as it's own icon
Add table sync iconBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a bit of a nit but could we either call this data-table-sync
or use the same underlying table icon with only one small column and a wider space to the right?
I'm thinking we may end up with a full set of data-table
icons and it would be nice to either give them a unique prefix, or just stick with the underlying table icon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decided against this - "Data table" is a bit redundant and there's no need to have a separate set of "data table" icons from "table" just because of the # of columns. # of columns can be dictated by what works best for the particular icon (ex see "Table: disconnect")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks for the update to add table-sync as a new icon!
revertBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
Fixes #0000
Checklist
Changes proposed in this pull request:
Update CSSStyle and add TableSync
Reviewers should focus on:
New icon designs.
Screenshot
Reach out to xinyifu on Slack for design nits.