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.

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

Add Confluent logo as webview icon #711

Merged
merged 3 commits into from
Dec 6, 2024
Merged

Conversation

noeldevelops
Copy link
Member

@noeldevelops noeldevelops commented Dec 5, 2024

Summary of Changes

  • A confluent logo as the webview "favicon" in the tabs we generate.
Screenshot 2024-12-06 at 11 42 11 AM

Any additional details or context that should be provided?

Light theme Dark theme
Screenshot 2024-12-05 at 4 23 35 PM Screenshot 2024-12-05 at 4 23 21 PM

Pull request checklist

Please check if your PR fulfills the following (if applicable):

Tests
  • Added new
  • Updated existing
  • Deleted existing
Other
  • All new disposables (event listeners, views, channels, etc.) collected as for eventual cleanup?
  • Does anything in this PR need to be mentioned in the user-facing CHANGELOG or README?
  • Have you validated this change locally by packaging and installing the extension .vsix file?
    gulp clicktest

@noeldevelops noeldevelops added the needs discussion Needs more input from the team(s) involved label Dec 5, 2024
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@shouples
Copy link
Contributor

shouples commented Dec 5, 2024

Quick mention that we also have https://github.com/confluentinc/vscode/blob/main/resources/icons/logo.svg used for the custom ThemeIcons, but maybe we need some general icon cleanup. 🤔

@noeldevelops
Copy link
Member Author

Quick mention that we also have https://github.com/confluentinc/vscode/blob/main/resources/icons/logo.svg used for the custom ThemeIcons, but maybe we need some general icon cleanup

@shouples agreed maybe a little icon cleanup is in order?
In the case of the one you linked, its currentcolor fill won't work since this icon ends up being used like an image file, so I had to make sure we had a pre-colored light & dark one. Webview also won't take a ThemeIcon type here which is unfortunate.

@noeldevelops noeldevelops removed the needs discussion Needs more input from the team(s) involved label Dec 6, 2024
@noeldevelops noeldevelops marked this pull request as ready for review December 6, 2024 18:46
@noeldevelops noeldevelops requested a review from a team as a code owner December 6, 2024 18:46
Copy link
Contributor

@shouples shouples left a comment

Choose a reason for hiding this comment

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

💯

@noeldevelops noeldevelops merged commit 877e478 into main Dec 6, 2024
2 checks passed
@noeldevelops noeldevelops deleted the ncothren/webview-custom-icon branch December 6, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants