Make title bar icons more visible, match brand colors#5582
Conversation
🦋 Changeset detectedLatest commit: 9bc7fb5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![kiloconnect[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
src/assets/icons/kilo-brand-dark.svg
Outdated
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <svg id="Kilo_Code_Branding" xmlns="http://www.w3.org/2000/svg" version="1.1" viewBox="0 0 39.9997627 39.9999944"> | |||
| <g transform="scale(0.8)" transform-origin="center"> | |||
There was a problem hiding this comment.
WARNING: SVG transform-origin on <g> may be ignored by some renderers
If transform-origin="center" isn’t honored, transform="scale(0.8)" will scale from (0,0), potentially shifting/cropping the icon in VS Code. Consider baking the transform into the viewBox/path data or using an explicit translate(...) scale(...) transform that centers deterministically.
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <svg id="Kilo_Code_Branding" xmlns="http://www.w3.org/2000/svg" version="1.1" viewBox="0 0 39.9997627 39.9999944"> | |||
| <g transform="scale(0.8)" transform-origin="center"> | |||
There was a problem hiding this comment.
WARNING: SVG transform-origin on <g> may be ignored by some renderers
If transform-origin="center" isn’t honored, transform="scale(0.8)" will scale from (0,0), potentially shifting/cropping the icon in VS Code. Consider baking the transform into the viewBox/path data or using an explicit translate(...) scale(...) transform that centers deterministically.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (5 files)
|
|
Light themes: 
Dark themes: 
Current: 
|
src/package.json
Outdated
| "light": "assets/icons/kilo-light.svg", | ||
| "dark": "assets/icons/kilo-dark.svg" | ||
| "light": "assets/icons/kilo-brand-light.svg", | ||
| "dark": "assets/icons/kilo-brand-dark.svg" |
There was a problem hiding this comment.
why not replace the existing icons? Or are they still used elsewhere? Having multiple seems bad to me for consistency?
There was a problem hiding this comment.
@markijbema - they are used elsewhere:
- The ChatView
- Commit message generation
- AgentManager panel
Let me try to swap them though and see how it looks. I was trying to go for a minimal change, but the color may help on the commit message generation as it's really hard to see otherwise.
There was a problem hiding this comment.
So here is a test, what do you think @markijbema? I'll ask Halyna (our designer) too.
My gut feel is:
- I like the updated commit message so it pops a bit more, I never knew it was there until someone told me. I think we change this for the same reason as the view title bar to open Kilo Code. (It's so easy to miss)
- I don't like the agent manager view title icon, it seems unnecessarily attention-grabbing given other view title icons. There is title text to provide the clarity.
- The agentmanager main page doesn't update currently, but can be adjusted.
- I'm ambivalent on the chatview change. I tried to make it animate for fun, but lottie is big. (>1MB)
Dark
### Light
There was a problem hiding this comment.
i actually like the agent manager, since that tab is buried between file tabs, so you might actually need the extra visibility. I'd vote for consistently changing.
I find the yellow icon in the chat a bit much, but then again, you're not often in that tab when you're actually using kilo, so I think it's fine
|
OK this is now updated everywhere: Dark mode
Light mode
|
|
@markijbema - I think this is ready for final review. Edit: Addressing the chatview feedback. |
| // kilocode_change start: KiloLogo component | ||
| const KiloLogo = () => { | ||
| const iconsBaseUri = (window as any).ICONS_BASE_URI || "" | ||
| const isLightTheme = /\bvscode-light\b|\bvscode-high-contrast-light\b/i.test(document.body.className) |
There was a problem hiding this comment.
WARNING: Theme detection via document.body.className is brittle and won’t react to theme changes
Two concerns here:
classNamestring parsing is a bit fragile vsclassList.contains(...).- This value is computed during render; if the user changes VS Code theme while the webview stays open, this component may not re-render, leaving the icon stuck on the old theme.
| const isLightTheme = /\bvscode-light\b|\bvscode-high-contrast-light\b/i.test(document.body.className) | |
| const isLightTheme = document.body.classList.contains("vscode-light") || document.body.classList.contains("vscode-high-contrast-light") |
| import React from "react" | ||
|
|
||
| export const KiloLogo = () => { | ||
| const isLightTheme = /\bvscode-light\b|\bvscode-high-contrast-light\b/i.test(document.body.className) |
There was a problem hiding this comment.
WARNING: Theme detection computed once; logo color may not update on theme switch
Same pattern as in the chat logo: this is evaluated during render, so if VS Code theme changes without a component re-render, the logo can remain with the old theme’s color. Also, classList.contains is less error-prone than regex on className.
| const isLightTheme = /\bvscode-light\b|\bvscode-high-contrast-light\b/i.test(document.body.className) | |
| const isLightTheme = document.body.classList.contains("vscode-light") || document.body.classList.contains("vscode-high-contrast-light") |
| @@ -1,6 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <svg id="Kilo_Code_Branding" xmlns="http://www.w3.org/2000/svg" version="1.1" viewBox="0 0 39.9997627 39.9999944"> | |||
| <g transform="scale(0.8)" transform-origin="center"> | |||
There was a problem hiding this comment.
WARNING: SVG transform-origin on <g> may be ignored
If transform-origin="center" isn’t honored, transform="scale(0.8)" will scale from (0,0), potentially shifting/cropping the icon in VS Code. Consider baking the transform into the viewBox/path data or using an explicit translate(...) scale(...) transform that centers deterministically.
| @@ -1,6 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <svg id="Kilo_Code_Branding" xmlns="http://www.w3.org/2000/svg" version="1.1" viewBox="0 0 39.9997627 39.9999944"> | |||
| <g transform="scale(0.8)" transform-origin="center"> | |||
There was a problem hiding this comment.
WARNING: SVG transform-origin on <g> may be ignored
If transform-origin="center" isn’t honored, transform="scale(0.8)" will scale from (0,0), potentially shifting/cropping the icon in VS Code. Consider baking the transform into the viewBox/path data or using an explicit translate(...) scale(...) transform that centers deterministically.
| const isLightTheme = | ||
| document.body.classList.contains("vscode-light") || | ||
| document.body.classList.contains("vscode-high-contrast-light") | ||
| const iconFile = isLightTheme ? "kilo-light.svg" : "kilo-dark.svg" |
There was a problem hiding this comment.
WARNING: Chat logo may not update on VS Code theme switch
The light/dark choice is computed during render. If the user changes theme while the webview remains open, this component may not re-render, leaving the icon stuck. Consider subscribing to theme changes (e.g., observing document.body class changes like the Agent Manager logo does).
There was a problem hiding this comment.
this needs the same logic as in KiloLogo i think? But cant we just use KiloLogo component here?
There was a problem hiding this comment.
@markijbema - there must be some other listener, as this one updates automatically as-is.
There was a problem hiding this comment.
- Extension-side listener (ClineProvider.ts:943-948): The VS Code extension listens for workbench.colorTheme configuration changes and sends a { type: "theme", ... } message to the webview.
- Context state update (ExtensionStateContext.tsx:458-462): When the webview receives this theme message, it calls setTheme(), updating the context state.
- Component re-render: The ChatView component uses useExtensionState() (line 110-133), so when any state in that context changes (including theme), the entire component re-renders.
|
|
||
| useEffect(() => { | ||
| const observer = new MutationObserver(() => { | ||
| setIsLightTheme(getIsLightTheme()) |
There was a problem hiding this comment.
This is a noop right? seems dubious
There was a problem hiding this comment.
oh wait, i see, can you please rename 'getIsLightTheme' to something like getIsLightThemeFromEditor or something, so it clearly isnt the react state?
| import React, { useState, useEffect } from "react" | ||
|
|
||
| const getIsLightThemeFromEditor = () => | ||
| document.body.classList.contains("vscode-light") || document.body.classList.contains("vscode-high-contrast-light") |
There was a problem hiding this comment.
WARNING: Potential document.body null access in getIsLightThemeFromEditor()
document.body is accessed during render; in some environments (early webview boot / non-browser harness) it can be unavailable and cause a hard crash. A defensive check keeps the component resilient.
| document.body.classList.contains("vscode-light") || document.body.classList.contains("vscode-high-contrast-light") | |
| !!(document.body && (document.body.classList.contains("vscode-light") || document.body.classList.contains("vscode-high-contrast-light"))) |
| const observer = new MutationObserver(() => { | ||
| setIsLightTheme(getIsLightThemeFromEditor()) | ||
| }) | ||
| observer.observe(document.body, { attributes: true, attributeFilter: ["class"] }) |
There was a problem hiding this comment.
WARNING: MutationObserver.observe can throw if document.body is unavailable
If document.body isn't ready for any reason, this call will throw. Guarding the observe call avoids an unhandled exception.
| observer.observe(document.body, { attributes: true, attributeFilter: ["class"] }) | |
| document.body && observer.observe(document.body, { attributes: true, attributeFilter: ["class"] }) |
2 participants
Context
We have been getting consistent reports that opening Kilo within VSCode is difficult. New users don't know where to look, and it can be easy to "lose" it in the variety of views and finding it again is not very easy.
One way to solve that has been adding the icon to the view title bar, but it blends in so well most users didn't even know it was added. (In fact it was added recently without most folks knowing in a recent Roo merge)
This change makes the icon pop more in the view bar, and should leave other icons alone. (I think). It uses our brand colors, so on dark themes it will be our green, and light themes our black'ish color.