-
Notifications
You must be signed in to change notification settings - Fork 31
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
added icon from getbootstrap #125
base: main
Are you sure you want to change the base?
Conversation
Waiting on #106 @niklasmtj 😏 |
Hey @davidkrupa first of all, thank you very much for your PR. Have you seen that the current Twitter Icon is a little different than the one provided by Bootstrap? You can see the current style in the following screenshot. So it would be great if the new X logo would also use this kind of style. If you need a hand building that let me know :) |
Hey @niklasmtj I found the icon in the same style. Taken from seeklogo.com |
@davidkrupa Hey! Sorry for reaching out that late. This looks good. One thing that i see in the changed files, is that the width and height is different. It would be great if you could have a look there. The current one is 32x32 px and your proposed change has 1000x1000 px. Let me know if you have any questions :) |
@niklasmtj The svg file didn't work, so I took a png and converted it to svg. It has a size of 1000 x 1000 pixels, but the file weight is only 2 KB, which is smaller than the Reddit icon. I think it's because it's a vector file, so it's downsized anyway. |
Yeah, that might be the case. However since the svg file is now 1333x1333 this could blow up the design on the pages. That was my initial request to update it back to the initial size of 32x32 px as you can see in the Plus, please have a look on the DCO action. The commits need to be signed to go through :) |
should we close this PR? @todaywasawesome @scottrigby ? |
I noted here, that deploy previews are a convenience and not a blocker for new PRs like this
|
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.
One thing that is absolutely needed is to fix DCO signoff on your commit. We can't merge commits without DCO signoff. See https://github.com/open-gitops/.github/blob/main/CONTRIBUTING.md#-licenses-and-copyright
Fix #124